T09 · Insecure Skill Coding Practices
Error
- Location
scripts/todo.py:213- Finding
OS Command Injection Through Unvalidated Reminder Job IDs
- Content
View full analysis
- Remediation
View remediation
Security audit
Todo List 待办事项管理
Security checks for vulnerabilities and agentic risk
Overview
This todo skill mostly matches its stated purpose, but it has a confirmed command-injection flaw in reminder cleanup and sends/stores chat routing data for reminders and status messages.
Review before installing. This skill can store todos, copy attachments, remember chat routing, send task lists/status to a configured chat target, and create scheduled reminders. It should be fixed to remove all shell=True reminder deletion calls, validate cron job IDs, harden attachment path handling, and require clearer confirmation for external sends and destructive actions.
Vulnerability Patterns
- Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
- Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
- Agent Memory PoisoningWrites attacker-controlled rules into memory that affect later sessions
- Remote Payload Retrieval and ExecutionFetches external code whose behavior can change after review
- Embedded Malicious CodeShips malicious scripts inside the skill and executes them locally
Findings (3)
T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/todo.py:361- Finding
Attachment Path Restrictions Can Be Bypassed With Symbolic Links
- Content
View full analysis
Optional[Dict]: """Add an attachment to a task. Security measures: - Only access a file explicitly specified by the user - Check that the file exists and is readable - Limit file size to 50 MB - Do not allow access to sensitive system directories """ # Security check: prevent access to sensitive system directories sensitive_dirs = ["/etc", "/root", "/var/log", "/var/lib", "/proc", "/sys"] abs_path = os.path.abspath(file_path) for sensitive_dir in sensitive_dirs: if abs_path.startswith(sensitive_dir): print(f"❌ Security restriction: access to system directory {sensitive_dir} is not allowed") return None if not os.path.exists(file_path): print(f"❌ File does not exist: {file_path}") return None if not os.access(file_path, os.R_OK): print(f"❌ File is not readable: {file_path}") return None # Check file size, limited to 50 MB file_size = os.path.getsize(file_path) max_size = 50 * 1024 * 1024 if file_size > max_size: print(f"❌ File is too large: {file_size / 1024 / 1024:.2f} MB") return None # Create a task-specific attachment directory task_attach_dir = os.path.join(ATTACHMENTS_DIR, task_id) os.makedirs(task_attach_dir, exist_ok=True) # Copy the file file_name = os.path.basename(file_path) dest_path = os.path.join(task_attach_dir, file_name) # Add a numeric suffix if the file name conflicts counter = 1 base_name, ext = os.path.splitext(file_name) while os.path.exists(dest_path): file_name = f"{base_name}_{counter}{ext}" dest_path = os.path.join(task_attach_dir, file_name) counter += 1 ...[truncated 2363 chars]- Remediation
View remediation
T09 · Insecure Skill Coding Practices
Note
- Location
scripts/merge-reminders.py:34- Finding
Reminder Migration Stores Job Names Instead of Actual Job IDs and Does Not Remove Existing Jobs
- Content
View full analysis
bool: """Create a cron reminder.""" session = load_session_context() if not session: print("❌ Error: missing session configuration") return False cmd = [ "openclaw", "cron", "add", "--name", name, "--at", at_time, "--channel", session["channel"], "--to", session["target"], "--message", message ] try: result = subprocess.run( cmd, capture_output=True, text=True, timeout=10 ) return result.returncode == 0 except Exception as e: print(f" ❌ Creation failed: {str(e)}") return False ``` The caller treats the Boolean result as success but records the job name as a job ID: ```python reminder_name = f"todo-merged-{bucket_time.replace(' ', '-').replace(':', '')}" job_ids = [] for label, reminder_time in reminder_times: cron_time = reminder_time.strftime("%H:%M %Y-%m-%d") full_name = f"{reminder_name}-{label}" if label == "30min": msg = "⏰ Todo reminder in 30 minutes:\n\n" + "\n".join(message_lines[1:]) elif label == "15min": msg = "⏰ Todo reminder in 15 minutes:\n\n" + "\n".join(message_lines[1:]) else: msg = "\n".join(message_lines) if create_cron_reminder(full_name, cron_time, msg): created_count += 1 job_ids.append(full_name) reminders_data["reminders"][reminder_name] = { "time_bucket": bucket_time, "task_ids": [t["id"] for t in bucket_tasks], "job_ids": job_ids, "created_at": datetime.now().isoformat() } ``` ### Technical Analysis `create_cron_reminder` returns only a Boolean and discards th ...[truncated 2083 chars]- Remediation
View remediation
Optional[str]: result = subprocess.run( cmd, capture_output=True, text=True, timeout=10, check=False ) if result.returncode != 0: return None response = json.loads(result.stdout) job_id = response.get("id") return job_id if isinstance(job_id, str) and job_id else None ``` Then append the returned ID rather than `full_name`. Additional hardening should include: 1. Enumerate existing scheduled jobs before migration. 2. Delete only jobs carrying an exact Skill-owned naming prefix or ownership marker. 3. Validate all IDs before passing them to OpenClaw. 4. Abort the rebuild if old jobs cannot be safely enumerated or removed. 5. Preserve existing reminder state until the new jobs have been created successfully. 6. Roll back newly created jobs if migration fails partway through. 7. Make repeated migration runs idempotent. 8. Update the documentation if automatic deletion is not actually supported. ]]>
Vulnerability Patterns
- Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
- Excessive AgencyUnrestricted Tool Access, Autonomous Decision Making, Scope Creep
- Tool MisuseTool Parameter Abuse, Chaining Abuse, Unsafe Defaults
- Rogue AgentSelf-Modification, Session Persistence
- Trigger AbuseOverly Broad Trigger, Shadow Command Trigger, Keyword Baiting Trigger
Findings (49)
Tool Parameter Abuse
High
- Category
- Tool Misuse
- Confidence
- 80% confidence
- Finding
Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).
- Content
md - **状态**: ✅ 已发布 - **Changelog**: `安全修复：移除shell=True，添加依赖声明，增强文件访问安全` - **修复内容**: 1. 移除 `subprocess.run(shell=True)` 使用，改用安全的 args 列表形式 2. 在 metadata 中声明 Python 3 和 OpenClaw CLI 依赖 3. 添加文件访问安全措施（禁止访问系统目录、文件大小限制、权限检查） 4. 添加详细的安全说明文档
Tool Parameter Abuse
High
- Category
- Tool Misuse
- Confidence
- 80% confidence
- Finding
Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).
- Content
⚠️ 安全修复详情
1. 命令注入风险修复
问题:
create_cron_job()使用subprocess.run(cmd, shell=True)修复: 改用 args 列表形式
python
Tool Parameter Abuse
High
- Category
- Tool Misuse
- Confidence
- 80% confidence
- Finding
Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).
- Content
⚠️ 安全修复详情
1. 命令注入风险修复
问题:
create_cron_job()使用subprocess.run(cmd, shell=True)修复: 改用 args 列表形式
python
Tp4
High
- Category
- MCP Tool Poisoning
- Confidence
- 97% confidence
- Finding
The skill documents commands that actively send status and list data to external chat targets and store channel/target identifiers for later reuse, but this behavior is not clearly reflected in the top-level description. That mismatch can cause users or calling agents to treat the skill as local-only task management when it actually performs outbound communication and session persistence.
- Content
Tp4
High
- Category
- MCP Tool Poisoning
- Confidence
- 97% confidence
- Finding
The skill documents commands that actively send status and list data to external chat targets and store channel/target identifiers for later reuse, but this behavior is not clearly reflected in the top-level description. That mismatch can cause users or calling agents to treat the skill as local-only task management when it actually performs outbound communication and session persistence.
- Content
Context-Inappropriate Capability
High
- Category
- Not specified by scanner
- Confidence
- 97% confidence
- Finding
The attachment feature explicitly supports copying, storing, and deleting user-specified local files, which exceeds the minimum capability needed for a todo manager and creates a path for unintended file access or destructive file operations. In the surrounding context, the skill also surfaces attachment paths in outputs, increasing the chance of sensitive local data exposure and harmful file handling.
- Content
Tool Parameter Abuse
High
- Category
- Tool Misuse
- Confidence
- 100% confidence
- Finding
This is a true tool-parameter abuse issue because untrusted old_job_id data is concatenated into a shell command. The todo skill stores and later reuses external identifiers, so compromised reminder state can turn routine reminder maintenance into arbitrary command execution.
- Content
python if reminder_name in reminders: for old_job_id in reminders[reminder_name].get("job_ids", []): try: subprocess.run(f"openclaw cron delete {old_job_id}", shell=True, capture_output=True) except: pass
Tool Parameter Abuse
High
- Category
- Tool Misuse
- Confidence
- 100% confidence
- Finding
This path abuses a tool parameter by passing job_id through a shell-interpreted command string. Because the parameter comes from stored reminder metadata, an attacker can potentially persist a payload that executes later when a bucket is deleted.
- Content
python # 删除所有关联的 cron 任务 for job_id in reminders[reminder_name].get("job_ids", []): try: subprocess.run(f"openclaw cron delete {job_id}", shell=True, capture_output=True) except: pass
Tool Parameter Abuse
High
- Category
- Tool Misuse
- Confidence
- 100% confidence
- Finding
The update-due workflow contains the same shell-based tool-parameter abuse, allowing malicious job IDs to escape into shell syntax. This is particularly dangerous because ordinary user actions like changing deadlines may trigger the vulnerable path.
- Content
python # 先删除旧的 cron for job_id in old_reminder.get("job_ids", []): try: subprocess.run(f"openclaw cron delete {job_id}", shell=True, capture_output=True) except: pass old_reminder["job_ids"] = []
Tool Parameter Abuse
High
- Category
- Tool Misuse
- Confidence
- 100% confidence
- Finding
This is another confirmed command-injection sink via tool parameter abuse: a stored job_id is inserted into a shell command during reminder bucket updates. In the context of a personal productivity skill, such hidden execution capability is more dangerous because users will not expect a todo action to execute arbitrary shell fragments.
- Content
python # 删除旧的 cron 任务 for job_id in new_reminder.get("job_ids", []): try: subprocess.run(f"openclaw cron delete {job_id}", shell=True, capture_output=True) except: pass
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 92% confidence
- Finding
This markdown file contains the skill's checklist and operational documentation exclusively in Chinese, including headings, instructions, and safety notes. Under the policy, forcing a specific language without user opt-in or a documented justification can be a natural-language policy violation.
- Content
Natural-Language Policy Violations
Medium
- Category
- Not specified by scanner
- Confidence
- 86% confidence
- Finding
The skill description is written entirely in Chinese and does not indicate that language selection is optional or that the skill is intended only for a Chinese-speaking or region-specific audience. Under the policy, forcing a specific language without user opt-in is a natural-language policy violation.
- Content
Undeclared Tool Scope
Medium
- Category
- MCP Least Privilege
- Confidence
- 96% confidence
- Finding
The skill invokes Python scripts that read and write files and use shell-accessible external tooling, yet the manifest declares no explicit tool scope or permissions boundary. This creates an authorization ambiguity where an agent may run capabilities broader than a user would reasonably expect, especially given attachment handling, persistent storage, and outbound messaging via OpenClaw.
- Content
Vague Triggers
Medium
- Category
- Not specified by scanner
- Confidence
- 91% confidence
- Finding
The trigger conditions include very broad everyday terms such as '待办', 'Todo', and direct variants that are likely to collide with ordinary conversation. Over-broad activation increases the chance the skill runs unexpectedly, which is more dangerous here because it can write persistent state and send messages to saved chat targets.
- Content
Missing User Warnings
Medium
- Category
- Not specified by scanner
- Confidence
- 95% confidence
- Finding
The direct-send section instructs the agent to send status or task lists to the current chat target and to persist channel/target details, but it does not present a prominent user-facing warning that task data will be transmitted externally. Users may assume the output is local to the assistant response rather than pushed through an external messaging path, creating confidentiality and consent risks.
- Content
Session Persistence
Medium
- Category
- Rogue Agent
- Confidence
- 60% confidence
- Finding
Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
- Content
bash # 创建项目 python3 todo.py project create "标签" "项目名称" [-d "描述"] # 列出项目 python3 todo.py project list
Session Persistence
Medium
- Category
- Rogue Agent
- Confidence
- 60% confidence
- Finding
Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
- Content
bash # 创建项目 python3 todo.py project create "标签" "项目名称" [-d "描述"] # 列出项目 python3 todo.py project list
Vague Triggers
Medium
- Category
- Not specified by scanner
- Confidence
- 96% confidence
- Finding
The trigger description is very broad and can match common conversation about tasks, making accidental activation likely. In skill systems, overbroad triggering is dangerous because it can cause the agent to enter a stateful tool flow and read or modify persisted user data without sufficiently clear intent.
- Content
Missing User Warnings
Medium
- Category
- Not specified by scanner
- Confidence
- 94% confidence
- Finding
Stating that reminders include attachment paths exposes local filesystem locations without any privacy warning or minimization. File paths often reveal usernames, project names, directory structure, or sensitive context even when file contents are not shown.
- Content
Ssd 3
Medium
- Category
- Not specified by scanner
- Confidence
- 95% confidence
- Finding
The skill specifies that reminders include attachment paths and that display behavior includes task details in plain-language outputs. This can leak sensitive local filesystem information and user-provided content into conversational channels where it may be exposed more broadly than intended.
- Content
Vague Triggers
Medium
- Category
- Not specified by scanner
- Confidence
- 98% confidence
- Finding
The invocation rule uses a catch-all for any request containing very common words like '待办', '任务', or 'todo', which can over-trigger in unrelated contexts. Because this skill can delete tasks, attach files, and manage reminders, ambiguous activation materially raises the risk of unintended data access or modification.
- Content
Context-Inappropriate Capability
Medium
- Category
- Not specified by scanner
- Confidence
- 94% confidence
- Finding
该技能宣称的用途是待办事项管理，但参考文档把核心使用方式定义为直接运行本地 Python 脚本并切换目录执行。这意味着技能依赖通用本地代码执行能力，而不是受限的待办接口；对一个待办管理技能而言，这种能力并非由其业务目的本身正当化。
- Content
Ssd 3
Medium
- Category
- Not specified by scanner
- Confidence
- 94% confidence
- Finding
The instructions explicitly say to reveal attachment information during task viewing/completion and to proactively mention or offer attachments. Proactive disclosure is risky because it can surface sensitive filenames and locations even when the user did not ask for them.
- Content
Missing User Warnings
Medium
- Category
- Not specified by scanner
- Confidence
- 92% confidence
- Finding
The documentation says deleting a task also deletes all attachment files, but it does not provide a prominent user warning that this is destructive and may be irreversible. Given the attachment model accepts local files, users may not understand the consequences and could lose data unexpectedly.
- Content
Ssd 3
Medium
- Category
- Not specified by scanner
- Confidence
- 95% confidence
- Finding
The task detail example includes a full local attachment path, and the documented reminder behavior surfaces attachment location context. This unnecessarily exposes filesystem layout and potentially sensitive identifiers embedded in paths, increasing privacy and reconnaissance risk.
- Content
Static analysis
No suspicious patterns detected.