T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/thread_repair.py:652
- Finding
- Shell Command Injection Through an Unvalidated Thread ID<![CDATA[ ## Vulnerability Details **File Location**: `scripts/thread_repair.py:652-673` **Vulnerability Type**: Shell command injection **Risk Level**: High ### Vulnerable Code ```python thread_id = result["thread"]["id"] output_path = Path(args.output).expanduser() if args.output else ( Path.home() / "Desktop" / f"repair-codex-thread-{thread_id[:8]}.command" ) output_path = output_path.resolve() if not output_path.parent.is_dir(): raise RepairError(f"Launcher parent directory does not exist: {output_path.parent}") if output_path.exists(): raise RepairError( f"Refusing to overwrite existing launcher: {output_path}; choose another --output path" ) log_path = output_path.with_suffix(".log") script_path = Path(__file__).resolve() command = " ".join( [ "/usr/bin/env", "python3", shlex.quote(str(script_path)), "--codex-home", shlex.quote(str(args.codex_home)), "apply", shlex.quote(thread_id), "--reopen", ] ) launcher = f"""#!/bin/bash set -uo pipefail export PATH=\"/usr/bin:/bin:/usr/sbin:/sbin:/opt/homebrew/bin\" readonly REPAIR_LOG={shlex.quote(str(log_path))} printf '\\n=== Codex thread repair: {thread_id} ===\\n' printf 'Press Cmd+Q in Codex before continuing.\\n' {command} 2>&1 | tee -a \"$REPAIR_LOG\" ``` ### Technical Analysis The thread ID is loaded from the local Codex SQLite database and inserted directly into a single-quoted shell command in the generated `.command` launcher: ```bash printf '\n=== Codex thread repair: {thread_id} ===\n' ``` Although the thread ID is safely quoted with `shlex.quote()` when used as an argument to the Python repair command, it is not escaped in the preceding `printf` statement. A thread ID containing a single quote followed by shell syntax can terminate the quoted string and introduce arbitrary commands. The project defines `SAFE_THREAD_ID_PATTERN`, but `command_prepare()` does not apply that validation before creating th ...[truncated 1674 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate the thread ID immediately after retrieving it and before using it in any filename, shell source, log output, or command: ```python thread_id = result["thread"]["id"] if ( not isinstance(thread_id, str) or not SAFE_THREAD_ID_PATTERN.fullmatch(thread_id) or thread_id in {".", ".."} ): raise RepairError(f"Unsafe thread ID: {thread_id!r}") ``` 2. Do not interpolate untrusted values into shell program text. Quote the value as a separate `printf` argument: ```python quoted_thread_id = shlex.quote(thread_id) launcher = f"""#!/bin/bash set -uo pipefail printf '\\n=== Codex thread repair: %s ===\\n' {quoted_thread_id} """ ``` 3. Prefer generating a launcher that contains only fixed shell syntax and passes all variable data as safely quoted positional arguments. 4. Add regression tests using IDs containing single quotes, command substitutions, semicolons, newlines, spaces, slashes, and redirection operators. Confirm that unsafe IDs are rejected before the launcher is written. 5. Apply the same validation at the database trust boundary in `resolve_thread()` so every downstream command receives a validated identifier. ]]>