T09 · Insecure Skill Coding Practices
Warning
- Location
- SKILL.md:102
- Finding
- Overbroad and Non-Atomic Session Index Deletion## Vulnerability Details **File Location**: `SKILL.md`, lines 102-111 **Vulnerability Type**: Destructive session-state modification caused by insufficient validation and unsafe file replacement **Risk Level**: Medium ### Vulnerable Code ```python if agent == 'main' and 'main' not in agent_ids: new_data = {} print(f"[{agent}] main 不存在，清空 ({len(data)} 条)") else: new_data = {k: v for k, v in data.items() if ':main' in k or v.get('status') == 'running'} print(f"[{agent}] 保留 {len(new_data)}，删 {len(data)-len(new_data)}") with open(sf, 'w') as f: json.dump(new_data, f) ``` ### Technical Analysis The cleanup algorithm retains only records whose key contains `:main` or whose status is exactly `running`. Consequently, it deletes every other entry without verifying that the session is actually expired, invalid, orphaned, or safe to remove. Sessions with unknown, absent, newly introduced, or temporarily inconsistent status values are treated as disposable. If the `main` agent is absent from the configuration, the code unconditionally replaces its entire session index with an empty object. It does not verify whether those sessions remain valuable, whether their files are still referenced elsewhere, or whether removal is consistent with the active Gateway state. The script then opens `sessions.json` directly in write mode. This truncates the original file before serialization completes. There is no mandatory backup, file lock, Gateway shutdown, temporary-file write, atomic rename, schema validation, or concurrency check. An interruption, serialization failure, or concurrent Gateway update can therefore leave the index corrupted or cause newly written session state to be lost. Although the documentation suggests creating a backup, backup creation is optional and is not enforced by the provided script. ### Attack Path 1. A user invokes the session-cleaner Skill and requests cleanup. 2. ...[truncated 1670 chars]
- Remediation
- ## Remediation Suggestions 1. **Use explicit eligibility rules:** Delete only sessions with recognized terminal states such as `done`, `timeout`, or `failed`, and only after validating retention-age requirements. Preserve records with missing or unknown statuses by default. 2. **Verify orphan status:** Before removing an index entry, verify whether its transcript and related files exist and whether another component still references the session. 3. **Require record-level confirmation:** Present the exact agent ID, session key, status, age, and files affected for every proposed deletion. Confirmation should apply to that immutable deletion plan rather than to a broad category. 4. **Create a mandatory backup:** Copy each `sessions.json` to a timestamped backup before modification and verify that the backup can be parsed. 5. **Use atomic replacement:** Serialize and validate the new index in the same directory, flush it to disk, and atomically replace the original with `os.replace`. 6. **Prevent concurrent writes:** Stop the Gateway before editing or use the locking or session-management API provided by OpenClaw. Confirm that the source index has not changed between analysis and replacement. 7. **Validate output before replacement:** Confirm that the generated JSON matches the expected schema and that protected sessions remain present. 8. **Avoid unconditional clearing:** Do not erase all `main` agent sessions solely because the agent is absent from the current configuration. Archive them or require a separate, explicit confirmation. 9. **Support recovery:** Keep backups until the Gateway restarts successfully and post-cleanup verification confirms that retained sessions are accessible. 10. **Align implementation and documentation:** The documentation claims that orphaned transcript, trajectory, and checkpoint files are deleted, but the supplied script only rewrites indexes. Either implement a separately reviewed and confirmed orphan-file cleanup p ...[truncated 26 chars]