From d3e29e0597613fde70171b115c8d6cae5531d1eb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 13:55:41 +0000 Subject: [PATCH] feat(research): scan the files a skill ships, not just the file it documents Step one measured the surface: 47.2% of a 290-skill sample name files no scan has ever read, 182 of them executable code. This reads them. The output that matters is the divergence set -- skills whose SKILL.md is clean but whose referenced code is not. That is hidden behaviour by construction: the documentation a reviewer or user sees says one thing, the code that runs says another. It is the only shape of finding worth calling malicious, and nothing in the pipeline could surface it before now. Two constraints, both learned the expensive way this fortnight: It does not convict. The rule engine is calibrated for SKILL.md prose; its false-positive profile on JavaScript and Python is unmeasured, and this project has already published one inflated number by assuming a rule meant what it appeared to mean. The report labels its own output as leads to read by hand. A 404 is not absence. A referenced path that fails to fetch is counted separately, because "names a script we could not retrieve" and "ships nothing" are different facts, and collapsing them is how a blind spot gets reported as larger than it is. Extraction moves into malwar.research.references so both scripts share one implementation with real test coverage. The tests are weighted toward negative cases -- every rule defect shipped here was a false positive found by hand afterwards, so the must-not-match set is deliberately larger than the must-match set and covers the shapes that fooled the detection rules: remote URLs, the user's own dotfiles, paths escaping the package, and prose that merely resembles a path. 1752 passing, 34 new. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DNoTXU8k3pfSBzR7aJubqL --- .github/workflows/measure-unread-surface.yml | 23 ++- scripts/extract-referenced-files.py | 75 +------- scripts/scan-referenced-files.py | 184 +++++++++++++++++++ src/malwar/research/__init__.py | 6 + src/malwar/research/references.py | 95 ++++++++++ tests/unit/test_references.py | 119 ++++++++++++ 6 files changed, 427 insertions(+), 75 deletions(-) create mode 100644 scripts/scan-referenced-files.py create mode 100644 src/malwar/research/__init__.py create mode 100644 src/malwar/research/references.py create mode 100644 tests/unit/test_references.py diff --git a/.github/workflows/measure-unread-surface.yml b/.github/workflows/measure-unread-surface.yml index 6cd44f6..b9f5f67 100644 --- a/.github/workflows/measure-unread-surface.yml +++ b/.github/workflows/measure-unread-surface.yml @@ -18,6 +18,11 @@ on: description: "Sampling seed, fixed so the number is reproducible" type: string default: "20260816" + mode: + description: "measure = count the surface; scan = fetch and scan those files" + type: choice + options: ["measure", "scan"] + default: "measure" permissions: contents: read @@ -35,8 +40,8 @@ jobs: with: python-version: "3.13" - - name: Install httpx - run: pip install httpx + - name: Install malwar + run: pip install -e "." && pip install httpx - name: Fetch the current snapshot run: | @@ -46,7 +51,8 @@ jobs: > data/registry-snapshots/latest.json echo "Snapshot: $(jq -r .created_at data/registry-snapshots/latest.json)" - - name: Measure + - name: Measure the surface + if: ${{ github.event.inputs.mode != 'scan' }} env: SAMPLE: ${{ github.event.inputs.sample }} SEED: ${{ github.event.inputs.seed }} @@ -56,6 +62,17 @@ jobs: --sample "$SAMPLE" --seed "$SEED" \ --json referenced-files.json + - name: Scan the referenced files + if: ${{ github.event.inputs.mode == 'scan' }} + env: + SAMPLE: ${{ github.event.inputs.sample }} + SEED: ${{ github.event.inputs.seed }} + run: | + python scripts/scan-referenced-files.py \ + data/registry-snapshots/latest.json \ + --sample "$SAMPLE" --seed "$SEED" \ + --json referenced-files.json + - name: Upload the path map if: always() uses: actions/upload-artifact@v4 diff --git a/scripts/extract-referenced-files.py b/scripts/extract-referenced-files.py index 35b63b7..045bf9f 100644 --- a/scripts/extract-referenced-files.py +++ b/scripts/extract-referenced-files.py @@ -31,79 +31,14 @@ import asyncio import json import random -import re import sys from collections import Counter from pathlib import Path from typing import Any -BASE_URL = "https://clawhub.ai/api/v1" - -# Interpreters and runners whose argument is a file the skill expects to run. -_RUNNERS = r"(?:node|python3?|bun|deno|ts-node|tsx|bash|sh|zsh|ruby|perl|php|Rscript)" - -# Ways a SKILL.md names a file it ships. Each must capture the path in group 1. -_PATTERNS: list[tuple[re.Pattern[str], str]] = [ - # Executed by an interpreter: `node scripts/foo.js`, `python3 ./bin/x.py`. - # ${SKILL_DIR}/ and ./ prefixes are stripped by _clean. - ( - re.compile( - rf"\b{_RUNNERS}\s+(?:\$\{{SKILL_DIR\}}/|\./)?" - r"([\w./-]+\.(?:js|mjs|cjs|ts|tsx|py|sh|bash|rb|pl|php|R))\b" - ), - "executed", - ), - # Explicit "Read `path`" instructions, which the agent will follow. - ( - re.compile(r"\b(?:Read|read|Load|load|See|see)\s+`([\w./-]+\.[\w]+)`"), - "read-instruction", - ), - # Markdown link or bare mention of a repo-relative source/doc file. - ( - re.compile( - r"[\(\[`\s](?:\./)?" - r"((?:scripts|bin|lib|src|tools|references|assets|specs)/[\w./-]+" - r"\.(?:js|mjs|cjs|ts|tsx|py|sh|bash|rb|pl|php|R|md|json|ya?ml|toml))" - ), - "referenced", - ), -] - -# Paths that are not part of the skill: the file we already scan, and things -# that belong to the user's project rather than the package. -_SKIP = re.compile( - r"^(?:SKILL\.md|README\.md|package(?:-lock)?\.json|tsconfig\.json" - r"|\.env(?:\.example)?|node_modules/.*|\.\./.*)$", - re.IGNORECASE, -) - - -def _clean(path: str) -> str | None: - """Normalise a captured path, or return None if it is not skill-local.""" - path = path.strip().strip("`'\"") - path = re.sub(r"^\$\{SKILL_DIR\}/", "", path) - path = re.sub(r"^\./", "", path) - if not path or path.startswith(("/", "~", "http")) or ".." in path: - return None - if _SKIP.match(path): - return None - # A bare filename with no directory and no extension we recognise is more - # likely prose than a shipped file. - if "." not in path: - return None - return path - - -def extract(text: str) -> dict[str, str]: - """Return {path: how it was referenced} for one SKILL.md.""" - found: dict[str, str] = {} - for pattern, kind in _PATTERNS: - for match in pattern.finditer(text): - cleaned = _clean(match.group(1)) - if cleaned and cleaned not in found: - found[cleaned] = kind - return found +from malwar.research.references import extract, is_executable +BASE_URL = "https://clawhub.ai/api/v1" async def sample_registry(slugs: list[str]) -> dict[str, str]: """Fetch SKILL.md for each slug. Requires network reach to the registry.""" @@ -196,11 +131,7 @@ def main() -> int: for ext, n in exts.most_common(12): print(f" {ext or '(none)':<10} {n:,}") - executable = sum( - 1 for v in refs.values() - for p in v if Path(p).suffix.lower() in - {".js", ".mjs", ".cjs", ".ts", ".tsx", ".py", ".sh", ".bash", ".rb", ".pl", ".php"} - ) + executable = sum(1 for v in refs.values() for p in v if is_executable(p)) print() print(f"of those, executable code: {executable:,}") diff --git a/scripts/scan-referenced-files.py b/scripts/scan-referenced-files.py new file mode 100644 index 0000000..1e066ba --- /dev/null +++ b/scripts/scan-referenced-files.py @@ -0,0 +1,184 @@ +#!/usr/bin/env python3 +"""Scan the files a skill ships, and report where they disagree with its docs. + +Step one measured the surface: 47% of a 290-skill sample name files no scan has +ever read, 182 of them executable. This reads them. + +The output that matters is the **divergence set**: skills whose SKILL.md is +clean but whose referenced code is not. That set is hidden behaviour by +construction -- the documentation a reviewer or user would read says one thing, +the code that actually runs says another -- and it is the only shape of finding +worth publishing as malicious. + +Two things this deliberately does not do: + +* It does not convict. The rule engine is tuned for SKILL.md prose; run against + JavaScript and Python it has an unmeasured false-positive profile, and this + project has already published one inflated number by assuming otherwise. + Findings here are leads to read by hand, and the report says so. +* It does not treat a 404 as absence of code. A referenced path that fails to + fetch is counted separately, because "the skill names a script we could not + retrieve" and "the skill ships nothing" are different facts. + +Usage: + scan-referenced-files.py --sample N [--json out.json] +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import random +import sys +from collections import Counter +from pathlib import Path +from typing import Any + +from malwar.research.references import extract, is_executable + +BASE_URL = "https://clawhub.ai/api/v1" + +# Paced to stay inside the registry's ~120 req/min limit. +_DELAY = 0.55 + + +async def _get(client: Any, slug: str, path: str) -> tuple[int, str]: + resp = await client.get( + f"{BASE_URL}/skills/{slug}/file", params={"path": path} + ) + return resp.status_code, resp.text + + +async def scan_skill(client: Any, slug: str) -> dict[str, Any] | None: + """Fetch a skill's SKILL.md and everything it names, scanning each.""" + from malwar.sdk import scan + + try: + status, skill_md = await _get(client, slug, "SKILL.md") + except Exception as exc: + return {"slug": slug, "error": f"{type(exc).__name__}: {exc}"} + if status != 200: + return None + await asyncio.sleep(_DELAY) + + doc = await scan(skill_md, file_name=f"{slug}/SKILL.md", use_llm=False, use_urls=False) + refs = extract(skill_md) + + files: list[dict[str, Any]] = [] + for path in refs: + try: + status, body = await _get(client, slug, path) + except Exception as exc: + files.append({"path": path, "status": "error", "detail": str(exc)[:120]}) + await asyncio.sleep(_DELAY) + continue + await asyncio.sleep(_DELAY) + if status != 200: + # Named but not retrievable. Recorded, never counted as clean. + files.append({"path": path, "status": status}) + continue + res = await scan(body, file_name=f"{slug}/{path}", use_llm=False, use_urls=False) + files.append({ + "path": path, + "status": 200, + "kind": refs[path], + "executable": is_executable(path), + "verdict": res.verdict, + "risk": res.risk_score, + "rules": sorted({f.rule_id for f in res.findings if not f.suppressed}), + "bytes": len(body), + }) + + return { + "slug": slug, + "doc_verdict": doc.verdict, + "doc_risk": doc.risk_score, + "doc_rules": sorted({f.rule_id for f in doc.findings if not f.suppressed}), + "referenced": files, + } + + +async def run(slugs: list[str]) -> list[dict[str, Any]]: + import httpx + + out: list[dict[str, Any]] = [] + async with httpx.AsyncClient(timeout=20.0, follow_redirects=True) as client: + for i, slug in enumerate(slugs, 1): + result = await scan_skill(client, slug) + if result: + out.append(result) + if i % 10 == 0: + print(f" ...{i}/{len(slugs)} skills", flush=True) + return out + + +def report(results: list[dict[str, Any]]) -> None: + fetched = [r for r in results if "referenced" in r] + with_refs = [r for r in fetched if r["referenced"]] + all_files = [f for r in with_refs for f in r["referenced"]] + ok = [f for f in all_files if f.get("status") == 200] + unreachable = [f for f in all_files if f.get("status") != 200] + + print(f"\nskills scanned: {len(fetched):,}") + print(f" naming referenced files: {len(with_refs):,}") + print(f"referenced paths tried: {len(all_files):,}") + print(f" fetched: {len(ok):,}") + print(f" unreachable (named, 404): {len(unreachable):,}") + + if ok: + print("\nverdicts on referenced files:") + for v, n in Counter(f["verdict"] for f in ok).most_common(): + print(f" {v:<12} {n:,}") + + # The divergence set: clean docs, flagged code. + divergent = [ + (r, f) + for r in with_refs + if r["doc_verdict"] in ("CLEAN", "UNKNOWN") + for f in r["referenced"] + if f.get("status") == 200 and f.get("verdict") not in ("CLEAN", "UNKNOWN") + ] + print(f"\n{'=' * 62}") + print(f"DIVERGENCE (SKILL.md clean, referenced file flagged): {len(divergent)}") + print(f"{'=' * 62}") + if not divergent: + print(" none in this sample.") + print(" A clean result here is a real result: it says the unread") + print(" surface is large and, in this sample, not hiding anything.") + for r, f in divergent[:40]: + rules = ", ".join(x.replace("MALWAR-", "") for x in f["rules"]) + print(f" {r['slug'][:34]:<34} {f['path'][:30]:<30} " + f"{f['verdict']:<11} risk={f['risk']:<4} [{rules}]") + + print("\nThese are leads, not verdicts: the rule engine is calibrated for") + print("SKILL.md prose and its false-positive profile on code is unmeasured.") + print("Every one needs reading by hand before it is called anything.") + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("snapshot", type=Path) + ap.add_argument("--sample", type=int, default=60) + ap.add_argument("--seed", type=int, default=20260816) + ap.add_argument("--json", type=Path) + args = ap.parse_args() + + skills = json.loads(args.snapshot.read_text(encoding="utf-8")).get("skills", {}) + # Reproducible sampling, not cryptography: the seed is published so anyone + # can redraw the same sample and check the result. + rng = random.Random(args.seed) # noqa: S311 + chosen = rng.sample(sorted(skills), min(args.sample, len(skills))) + print(f"sampling {len(chosen)} skills (seed {args.seed})\n") + + results = asyncio.run(run(chosen)) + report(results) + + if args.json: + args.json.write_text(json.dumps(results, indent=2), encoding="utf-8") + print(f"\nwrote {args.json}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/malwar/research/__init__.py b/src/malwar/research/__init__.py new file mode 100644 index 0000000..c776d7a --- /dev/null +++ b/src/malwar/research/__init__.py @@ -0,0 +1,6 @@ +"""Research tooling that reaches beyond a single scanned file. + +The sweep scans one file per skill (SKILL.md). Everything here exists to look +at what a skill actually ships alongside that file, which is where behaviour +diverging from the documentation would live. +""" diff --git a/src/malwar/research/references.py b/src/malwar/research/references.py new file mode 100644 index 0000000..db02afc --- /dev/null +++ b/src/malwar/research/references.py @@ -0,0 +1,95 @@ +"""Extract the files a SKILL.md names, so the rest of the package can be read. + +The sweep scans one file per skill. A skill whose SKILL.md is clean and whose +``scripts/*.js`` beacons out is invisible to that scan, to a reviewer skimming +the listing, and to the user -- which is the definition of hidden behaviour and +the only place a real supply-chain payload would sit. + +Extraction is deliberately conservative: a path is reported only when the skill +*names* it. Guessing at conventional layouts (``scripts/*``, ``bin/*``) would +produce a larger figure built partly on files that do not exist, and an +inflated denominator is how you publish a blind spot bigger than the one you +have. +""" + +from __future__ import annotations + +import re + +# Interpreters and runners whose argument is a file the skill expects to run. +_RUNNERS = r"(?:node|python3?|bun|deno|ts-node|tsx|bash|sh|zsh|ruby|perl|php|Rscript)" + +# Extensions that are executable code, as opposed to prose or config. +EXECUTABLE_SUFFIXES: frozenset[str] = frozenset( + {".js", ".mjs", ".cjs", ".ts", ".tsx", ".py", ".sh", ".bash", ".rb", ".pl", ".php", ".r"} +) + +# How a SKILL.md names a file it ships. Each pattern captures the path in +# group 1. Order matters: the first match wins, so the strongest label (a file +# the agent is told to execute) is checked before the weakest (a mention). +_PATTERNS: list[tuple[re.Pattern[str], str]] = [ + ( + re.compile( + rf"\b{_RUNNERS}\s+(?:\$\{{SKILL_DIR\}}/|\./)?" + r"([\w./-]+\.(?:js|mjs|cjs|ts|tsx|py|sh|bash|rb|pl|php|R))\b" + ), + "executed", + ), + ( + re.compile(r"\b(?:Read|read|Load|load|See|see)\s+`([\w./-]+\.[\w]+)`"), + "read-instruction", + ), + ( + re.compile( + r"[\(\[`\s](?:\./)?" + r"((?:scripts|bin|lib|src|tools|references|assets|specs)/[\w./-]+" + r"\.(?:js|mjs|cjs|ts|tsx|py|sh|bash|rb|pl|php|R|md|json|ya?ml|toml))" + ), + "referenced", + ), +] + +# Not part of the skill package: the file already scanned, and files belonging +# to the user's own project rather than to the skill. +_SKIP = re.compile( + r"^(?:SKILL\.md|README\.md|package(?:-lock)?\.json|tsconfig\.json" + r"|\.env(?:\.example)?|node_modules/.*)$", + re.IGNORECASE, +) + + +def clean_path(path: str) -> str | None: + """Normalise a captured path, or return None when it is not skill-local. + + Anything absolute, user-home-relative, remote, or escaping the package with + ``..`` is rejected: those are not files the registry would serve for this + skill, and counting them would inflate the surface with paths that cannot + be fetched. + """ + path = path.strip().strip("`'\"") + path = re.sub(r"^\$\{SKILL_DIR\}/", "", path) + path = re.sub(r"^\./", "", path) + if not path or path.startswith(("/", "~", "http")) or ".." in path: + return None + if _SKIP.match(path): + return None + if "." not in path: + return None + return path + + +def extract(text: str) -> dict[str, str]: + """Return ``{path: how it was referenced}`` for one SKILL.md.""" + found: dict[str, str] = {} + for pattern, kind in _PATTERNS: + for match in pattern.finditer(text): + cleaned = clean_path(match.group(1)) + if cleaned and cleaned not in found: + found[cleaned] = kind + return found + + +def is_executable(path: str) -> bool: + """True when the path looks like code rather than prose or config.""" + dot = path.rfind(".") + return dot != -1 and path[dot:].lower() in EXECUTABLE_SUFFIXES diff --git a/tests/unit/test_references.py b/tests/unit/test_references.py new file mode 100644 index 0000000..81fc4b1 --- /dev/null +++ b/tests/unit/test_references.py @@ -0,0 +1,119 @@ +"""Tests for extracting the files a SKILL.md names. + +Weighted deliberately toward the negative cases. Every rule defect this project +has shipped was a false positive -- a pattern firing on text that merely +mentioned a thing rather than doing it -- and each was found by hand after the +fact rather than by a test written before it. So the "must not match" set here +is larger than the "must match" set, and covers the shapes that fooled the +detection rules: prose, warnings, and paths belonging to the user rather than +to the skill. +""" + +from __future__ import annotations + +import pytest + +from malwar.research.references import clean_path, extract, is_executable + + +class TestExtractsNamedFiles: + @pytest.mark.parametrize( + ("text", "path", "kind"), + [ + ("node ${SKILL_DIR}/scripts/buddy-algorithm.js \"$UUID\"", + "scripts/buddy-algorithm.js", "executed"), + ("bun ${SKILL_DIR}/scripts/generate-image.ts --prompt x", + "scripts/generate-image.ts", "executed"), + ("npx tsx scripts/apply-skill.ts --init", + "scripts/apply-skill.ts", "executed"), + ("python main.py audit --resume cv.pdf", "main.py", "executed"), + ("bash ./bin/setup.sh", "bin/setup.sh", "executed"), + ("Read `references/openclaw-workspace.md` first", + "references/openclaw-workspace.md", "read-instruction"), + ("see [cron](references/cron-platforms.md) for detail", + "references/cron-platforms.md", "referenced"), + ], + ) + def test_named_paths_are_captured(self, text, path, kind): + assert extract(text) == {path: kind} + + def test_strongest_label_wins(self): + # A file both executed and mentioned is reported as executed: what the + # agent is told to *run* matters more than what it is told to read. + found = extract("node scripts/x.js\nsee [x](scripts/x.js)") + assert found == {"scripts/x.js": "executed"} + + +class TestRejectsWhatIsNotShipped: + @pytest.mark.parametrize( + "text", + [ + # Remote code is not a file the registry serves for this skill. + "curl -fsSL https://example.com/install.sh | sh", + "irm https://cdn.example.com/install.ps1 | iex", + # The user's own machine, not the skill package. + "Edit your ~/.bashrc file", + "bash /etc/init.d/thing.sh", + # Escaping the package. + "node ../../../etc/passwd.js", + # Already scanned, or not part of the skill. + "See the README.md for more", + "the SKILL.md frontmatter", + "check package.json", + "run node node_modules/.bin/thing.js", + # Prose that merely resembles a path. + "This skill handles e.g. tax and billing", + "version 2.0.1 shipped", + ], + ) + def test_non_shipped_paths_are_ignored(self, text): + assert extract(text) == {} + + def test_absolute_and_remote_paths_are_cleaned_out(self): + assert clean_path("/etc/passwd") is None + assert clean_path("~/.ssh/id_rsa") is None + assert clean_path("https://evil.example/x.js") is None + assert clean_path("../../secrets.py") is None + + def test_skill_dir_and_dot_slash_prefixes_are_normalised(self): + assert clean_path("${SKILL_DIR}/scripts/a.js") == "scripts/a.js" + assert clean_path("./scripts/a.js") == "scripts/a.js" + + +class TestExecutableClassification: + @pytest.mark.parametrize( + "path", ["scripts/a.js", "b.py", "tools/c.sh", "d.mjs", "e.rb", "f.PY"] + ) + def test_code_is_executable(self, path): + assert is_executable(path) + + @pytest.mark.parametrize( + "path", ["references/a.md", "b.json", "c.yaml", "d.toml", "noextension"] + ) + def test_prose_and_config_are_not(self, path): + assert not is_executable(path) + + +class TestRealWorldSkillFiles: + """Whole-file behaviour on text taken from live skills.""" + + def test_buddy_card_names_both_scripts(self): + text = ( + "BUDDY_JSON=$(node ${SKILL_DIR}/scripts/buddy-algorithm.js \"$UUID\")\n" + "bun ${SKILL_DIR}/scripts/generate-image.ts --prompt \"

\" --image out.jpg\n" + "If bun is not installed, use: `npx -y bun ${SKILL_DIR}/scripts/generate-image.ts ...`\n" + ) + assert extract(text) == { + "scripts/buddy-algorithm.js": "executed", + "scripts/generate-image.ts": "executed", + } + + def test_a_skill_that_ships_nothing_yields_nothing(self): + # The common case. A skill that is purely instructions must not be + # counted toward the unread surface. + text = ( + "# Code Formatter\n\n" + "Format the user's code. Ask before rewriting whole files.\n" + "Prefer the project's existing style over your own.\n" + ) + assert extract(text) == {}