Back to skill

Security audit

cjg-skill-sync

Security checks for vulnerabilities and agentic risk

Overview

This skill is a real skill updater, but its update and backup safeguards do not fully match its safety promises and could overwrite or lose local skill changes.

Install only if you are comfortable with a skill updater that can modify installed skill directories. Prefer running check with --no-baseline first, inspect proposed changes, avoid --execute and autorun until the backup failure handling and unknown-baseline behavior are fixed, and keep your own backup of customized skills.

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

Error
Location
scripts/skill_sync.py:337
Finding
Destructive update proceeds after an incomplete backup<![CDATA[ ## Vulnerability Details **File Location**: `scripts/skill_sync.py:337-352` **Vulnerability Type**: Improper error handling during backup creation **Risk Level**: High ### Complete Code Snippet ```python dest = os.path.join(BACKUP_ROOT, slug, ts) os.makedirs(dest, exist_ok=True) for entry in os.listdir(slug_dir): if entry == SYNC_META: continue s, d = os.path.join(slug_dir, entry), os.path.join(dest, entry) try: shutil.copytree(s, d) if os.path.isdir(s) else shutil.copy2(s, d) except Exception as e: print(f" ⚠ 备份 {entry} 失败:{type(e).__name__}: {str(e)[:80]}") # 指针也写外部(写技能目录内会被 --force 一起删掉) try: os.makedirs(os.path.join(BACKUP_ROOT, slug), exist_ok=True) open(os.path.join(BACKUP_ROOT, slug, "last_backup.txt"), "w").write(dest) except Exception: pass ``` ### Technical Analysis The backup process catches errors for each file or directory, logs the failure, and continues. It then records the potentially incomplete directory as the latest backup. No success result, manifest comparison, file-count check, or hash verification is required before the caller proceeds. The update path subsequently invokes platform installation commands with `--force`. Those commands may replace the entire existing skill directory. Consequently, the claimed invariant that an update always has a complete rollback point is not enforced. Failures can result from unreadable files, insufficient storage, filesystem errors, path conflicts, interrupted copies, or destination collisions. This issue does not require elevated privileges, but it can destroy any files that the invoking user can modify in an installed skill directory. ### Attack Path 1. An installed skill contains one or more files that cannot be copied successfully, or the backup destination becomes unavailable or full. 2. The user runs `apply --execute`, or explicitly enables a scheduled `autorun --execute`. 3. `backup_dir()` catches the copy exception and contin ...[truncated 693 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Make `backup_dir()` fail closed: any copy failure must abort the backup and prevent installation. - Delete or quarantine partial backup directories after an error. - Generate a source manifest containing relative paths, file types, sizes, and SHA-256 hashes. - Compare the completed backup against that manifest before marking it valid. - Write `last_backup.txt` only after successful verification. - Use a temporary backup directory and atomically rename it after verification. - Propagate a structured success or failure result to `cmd_apply()`. - Refuse to invoke any force-installation command unless a verified backup exists. - Add tests covering unreadable files, disk exhaustion, interrupted copies, and partial directory-copy failures. ]]>

T09 · Insecure Skill Coding Practices

Error
Location
scripts/skill_sync.py:655
Finding
Unknown installation state is incorrectly promoted to clean without user confirmation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/skill_sync.py:655-665` **Vulnerability Type**: Unsafe trust-baseline initialization **Risk Level**: High ### Complete Code Snippet ```python if t["state"] == "unknown" and not args.force_overwrite: # 「从未建过快照」不等于「改过文件」——首次运行无从判断, # 让用户为此输 --force-overwrite 是糟糕体验且并无额外安全。 # 与 check 的行为保持一致:补建快照后视为未改动, # 真正的兜底是备份 + rollback(可逆)。 try: write_baseline(t["dir"]) print(" ℹ 首次运行无安装快照,已建当前快照并视为未改动(备份+回滚兜底)") except Exception as e: print(f" ⏭ 跳过:无法建立快照({type(e).__name__}: {str(e)[:80]})") continue ``` ### Technical Analysis A missing or unreadable baseline means the program has no trusted reference with which to distinguish original installation files from user-modified files. The safe state is therefore `unknown`, not `clean`. The implementation creates a baseline from the current files immediately before the update and then proceeds as though those files were an unmodified installation. This does not establish integrity: it merely records the potentially modified state. Existing customizations can therefore be overwritten without `--force-overwrite` or a separate confirmation. This behavior also conflicts with the documentation, which states that an absent baseline is handled conservatively as a possible local modification. ### Attack Path 1. A user customizes an installed skill before this updater creates a baseline, or the `.cjg_sync/baseline.json` file is removed or corrupted. 2. `check_local_changes()` reports the state as `unknown`. 3. The user invokes `apply --slug <skill> --execute`, believing local changes will be protected. 4. The updater records the current customized files as a new baseline. 5. It proceeds to create a backup and run the platform CLI with force-replacement behavior. 6. The customized files are overwritten without requiring `--force-overwrite` or explicit confirmation. ### Impact Assessment The issue can destroy loc ...[truncated 275 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Preserve `unknown` as a separate, unsafe state; never automatically convert it to `clean`. - Require explicit interactive confirmation or `--force-overwrite` before updating an unknown installation. - For non-interactive `autorun`, always skip unknown installations. - Prefer obtaining a trusted manifest from the original signed package or installation platform. - Bind baselines to the installed version and trusted package identity. - Detect baseline deletion or corruption and report it prominently rather than silently rebuilding it. - Update the implementation to match the documented conservative behavior. - Add tests proving that missing, malformed, and deleted baselines block unattended updates. ]]>

T09 · Insecure Skill Coding Practices

Warning
Location
scripts/skill_sync.py:615
Finding
Locally newer skills can be selected for downgrade<![CDATA[ ## Vulnerability Details **File Location**: `scripts/skill_sync.py:615-625` **Vulnerability Type**: Improper update-state validation **Risk Level**: Medium ### Complete Code Snippet ```python info = latest.get(slug) or {} new = info.get("latest_version") if not new: continue kind = classify(meta["version"], new) if kind == "latest": continue state, changed = check_local_changes(meta["dir"]) targets.append({"slug": slug, "dir": meta["dir"], "cur": meta["version"], "new": new, "kind": kind, "state": state, "changed": changed, "info": info}) ``` ### Technical Analysis The `classify()` function can return `ahead` when the installed version is newer than the version returned by the remote index. However, the update selection code excludes only `latest`. It therefore adds `ahead` entries to the update target list. Although `--safe-only` later filters these entries out, a targeted or general apply without `--safe-only` can continue to the installation stage. For ClawHub installations, the remotely supplied target version is passed to the CLI together with `--force`, enabling a downgrade. This can be triggered by stale remote metadata, a compromised version-index service, or ordinary synchronization delays. Update logic should never interpret `ahead` as an available update. ### Attack Path 1. The installed skill is version `2.0.0`. 2. The configured remote version service returns `1.9.0` as `latest_version` and supplies ClawHub installation metadata. 3. `classify("2.0.0", "1.9.0")` returns `ahead`. 4. Because only `latest` is excluded, the item is appended to the update targets. 5. The user runs `apply --slug <skill> --execute`. 6. The generated ClawHub command requests version `1.9.0` with `--force`. 7. The newer local release may be replaced by the older package. ### Impact Assessment A downgrade can reintroduce corrected vulnerabilities, remove compatibility fixes, or replace newer behavior with outdated c ...[truncated 232 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Only append targets whose classification is `patch`, `minor`, or explicitly approved `major`. - Reject `ahead`, `no_index`, `stalled`, and unknown classifications before backup or installation. - Add an explicit `--allow-downgrade` option if downgrades are a legitimate recovery feature. - Require interactive confirmation and a verified backup for any intentional downgrade. - Authenticate remote update metadata and validate that the selected version is newer than the installed version. - Add regression tests in which the remote index is stale and confirm that no installation command is generated. ]]>
Vulnerability Patterns
  • Rogue AgentSelf-Modification, Session Persistence
  • Trigger AbuseOverly Broad Trigger, Shadow Command Trigger, Keyword Baiting Trigger
  • Behavioral ASTexec() Call, eval() Call, Dynamic Import
  • Taint TrackingDirect Taint Flow, Variable-Mediated Taint Flow, Credential Exfiltration Chain
  • MCP Least PrivilegeUnderdeclared Capability, Wildcard Permission, Missing Permission Declaration
Findings (13)

Lp3

Medium
Category
MCP Least Privilege
Confidence
95% confidence
Finding
The skill advertises and instructs use of file reads/writes, network access, and shell execution, but it does not declare any explicit tool scope such as permissions or allowed-tools. That mismatch weakens least-privilege controls and makes it harder for a host to constrain dangerous operations like modifying local files, invoking platform CLIs, or contacting remote update sources. In this context, the risk is elevated because the skill's core function is to inspect installed skills and optionally execute update commands, which are inherently sensitive operations.

Vague Triggers

Medium
Confidence
95% confidence
Finding
L006 的触发词包含“更新技能”“升级技能”“技能版本”等泛化表达,缺少明确上下文约束或排除条件。这些短语在普通对话中也可能自然出现,容易让技能在非预期场景下被唤起。

subprocess module call

Medium
Category
Dangerous Code Execution
Content
print("\n=== 7) 离线可用性(不联网也能检查) ===")
try:
    import subprocess
    r = subprocess.run([sys.executable, os.path.join(HERE, "skill_sync.py"),
                        "check", "--offline", "--no-baseline"],
                       capture_output=True, text=True, timeout=60,
                       cwd=os.path.dirname(SKILL_DIR))
Confidence
70% confidence
Finding
subprocess module calls execute external commands. Without careful input validation, this enables command injection.

subprocess module call

Medium
Category
Dangerous Code Execution
Content
try:
    import subprocess
    exe = [sys.executable, os.path.join(HERE, "skill_sync.py")]
    r0 = subprocess.run(exe + ["autorun", "--help"], capture_output=True,
                        text=True, timeout=60)
    check("autorun 子命令已注册", r0.returncode == 0, r0.stderr[-120:])
    r = subprocess.run(exe + ["autorun"], capture_output=True, text=True,
Confidence
70% confidence
Finding
subprocess module calls execute external commands. Without careful input validation, this enables command injection.

subprocess module call

Medium
Category
Dangerous Code Execution
Content
r0 = subprocess.run(exe + ["autorun", "--help"], capture_output=True,
                        text=True, timeout=60)
    check("autorun 子命令已注册", r0.returncode == 0, r0.stderr[-120:])
    r = subprocess.run(exe + ["autorun"], capture_output=True, text=True,
                       timeout=120, cwd=os.path.dirname(SKILL_DIR))
    check("autorun 退出码 0", r.returncode == 0, r.stderr[-160:])
    out = r.stdout or ""
Confidence
70% confidence
Finding
subprocess module calls execute external commands. Without careful input validation, this enables command injection.

Session Persistence

Medium
Category
Rogue Agent
Content
设计铁律(勿改):
  1. 更新前必备份,失败必回滚。
  2. 检测到本地改动 → 不覆盖,先问(--force-overwrite 才强制)。
  3. 主版本变化 → 不自动执行(--allow-major 才允许)。
  4. 默认不联网改状态;每日自动检查默认关(opt-in)。
零第三方依赖(仅标准库),离线也能跑 check 的扫描部分。
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.

Session Persistence

Medium
Category
Rogue Agent
Content
设计铁律(勿改):
  1. 更新前必备份,失败必回滚。
  2. 检测到本地改动 → 不覆盖,先问(--force-overwrite 才强制)。
  3. 主版本变化 → 不自动执行(--allow-major 才允许)。
  4. 默认不联网改状态;每日自动检查默认关(opt-in)。
零第三方依赖(仅标准库),离线也能跑 check 的扫描部分。
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.

Tainted flow: 'd' from open (line 171, file read) → shutil.copy2 (file write)

Medium
Category
Data Flow
Content
continue
        s, d = os.path.join(slug_dir, entry), os.path.join(dest, entry)
        try:
            shutil.copytree(s, d) if os.path.isdir(s) else shutil.copy2(s, d)
        except Exception as e:
            print(f"  ⚠ 备份 {entry} 失败:{type(e).__name__}: {str(e)[:80]}")
    # 指针也写外部(写技能目录内会被 --force 一起删掉)
Confidence
65% confidence
Finding
Data from a source is assigned to a variable that is later passed to a sink, creating a variable-mediated taint flow.

Tainted flow: 'd' from open (line 171, file read) → shutil.copy2 (file write)

Medium
Category
Data Flow
Content
shutil.rmtree(p) if os.path.isdir(p) else os.remove(p)
    for entry in os.listdir(backup):
        s, d = os.path.join(backup, entry), os.path.join(slug_dir, entry)
        shutil.copytree(s, d) if os.path.isdir(s) else shutil.copy2(s, d)
    print(f"  ↩ 已回滚到 {backup}")
    return True
Confidence
82% confidence
Finding
Rollback restores all entries from a backup directory into the live skill directory without validating for symlinks, path safety, or file types. If an attacker can place crafted content in the backup location, restore_dir() can overwrite arbitrary files within the skill tree or copy symlinked content in unsafe ways, making rollback a file-write primitive.

subprocess module call

Medium
Category
Dangerous Code Execution
Content
def run_install(item, timeout=180):
    """真实执行安装命令(仅当显式 --execute)。返回 (ok, 输出尾部)。"""
    try:
        r = subprocess.run(item["cmd"], cwd=item["cwd"] or None,
                           capture_output=True, text=True, timeout=timeout,
                           encoding="utf-8", errors="replace")
        out = ((r.stdout or "") + (r.stderr or "")).strip()
Confidence
85% confidence
Finding
The script executes installer commands derived from remote metadata via plan_install() when --execute is used. Although subprocess.run is invoked with a list rather than shell=True, the executable path and arguments are still influenced by untrusted server-provided install_urls, so a compromised registry or malicious skill metadata could trigger execution of attacker-chosen local tools or installation of an untrusted package.

Description-Behavior Mismatch

Medium
Confidence
97% confidence
Finding
The check command is documented as 'only checks' and 'does not modify any files', but it silently creates baseline metadata in installed skill directories unless --no-baseline is supplied. This trust-boundary violation can mislead users and automation into granting the script write access during what appears to be a read-only operation.

Intent-Code Divergence

Medium
Confidence
93% confidence
Finding
The inline comment admits that check writes .cjg_sync metadata, while user-facing documentation promises check will not touch files at all. This inconsistency is security-relevant because operators may run the script in sensitive environments under false assumptions about its side effects.

Natural-Language Policy Violations

Low
Confidence
77% confidence
Finding
该文件标题、描述和操作说明主要以中文呈现,且未声明可根据用户偏好切换语言,也未说明这是面向特定中文区域用户的限定技能。按规则,这可能构成未获用户选择即强制特定语言/locale 的自然语言策略问题。

Static analysis

No suspicious patterns detected.