T09 · Insecure Skill Coding Practices
- Location
scripts/codex_history_manager.py:695- Finding
Dangerous edit plan identifier is not recomputed before history modification
- Content
View full analysis
tuple[int, int]: find = str(plan["find"]) replace = str(plan["replace"]) title, title_count = replace_literal(thread.title, find, replace) first_user_message, first_count = replace_literal(thread.first_user_message, find, replace) conn.execute( "update threads set title = ?, first_user_message = ?, updated_at = ? where id = ?", (title, first_user_message, now_timestamp(), thread.id), ) conn.commit() ``` ```python plan_path = args.plan.expanduser().resolve() plan = json.loads(plan_path.read_text(encoding="utf-8")) plan_id = plan.get("plan_id") if not isinstance(plan_id, str) or plan_id != args.confirm_plan_id: raise CodexHistoryError("Confirmation failed: --confirm-plan-id doe ...[truncated 3237 chars]- Remediation
View remediation
str: plan_core = { "thread_id": plan["thread"]["id"], "rollout_path": plan["thread"]["rollout_path"], "find": plan["find"], "replace": plan["replace"], "change_count": plan["change_count"], "total_occurrences": plan["total_occurrences"], } return hashlib.sha256( json.dumps( plan_core, ensure_ascii=False, sort_keys=True, separators=(",", ":"), ).encode("utf-8") ).hexdigest() ``` 2. Reject application unless all three values match: - The identifier recomputed from the loaded plan - The identifier stored in the plan - The identifier supplied through `--confirm-plan-id` 3. Use the complete SHA-256 digest rather than truncating it to 16 hexadecimal characters. 4. Regenerate the dangerous-edit plan against the current SQLite row and rollout file immediately before writing. Compare the complete change list and occurrence counts with those approved in the saved plan. 5. Include hashes of the source database fields and rollout file in the plan. Refuse application if either source changed after planning. 6. Validate plan field types strictly rather than coercing arbitrary values through `str()`. 7. If plan files may cross trust boundaries, authenticate them with an HMAC stored separately from the plan or a digital signature. A plain hash only detects changes when the application independently recomputes and verifies it. ]]>
