diff --git a/README.md b/README.md index a6c0545..d7e0c55 100644 --- a/README.md +++ b/README.md @@ -361,7 +361,7 @@ make profile-reset # go to the default profile (full) make new-skill name=myskill # scaffold agent + skill files ``` -`doctor.sh` checks: symlinks, GStack submodule, Playwright browser cache, prerequisites (git, Node, Cargo, Python, Claude Code), plugins, permissions, token budget, config consistency. +`doctor.sh` checks: symlinks, GStack submodule, vendored skills (curl-pinned externals in `plugins.lock.json` + `link.sh`'s `EXTERNAL_SKILLS`, per the active profile), Playwright browser cache, prerequisites (git, Node, Cargo, Python, Claude Code), plugins, permissions, token budget, config consistency. --- diff --git a/doctor.sh b/doctor.sh index 48cc977..122bf71 100644 --- a/doctor.sh +++ b/doctor.sh @@ -22,6 +22,8 @@ VERSION=$(cat "$REPO/version.txt" 2>/dev/null || echo "unknown") source "$REPO/lib/detect-plugins.sh" # shellcheck source=lib/gstack-playwright.sh disable=SC1091 source "$REPO/lib/gstack-playwright.sh" +# shellcheck source=lib/doctor-vendored.sh disable=SC1091 +source "$REPO/lib/doctor-vendored.sh" echo "" echo "═══ claude-config doctor (v${VERSION}) ═══" @@ -117,6 +119,38 @@ fi echo "" +# ──────────────────────────────────────────────────────────── +# 2b. Vendored skills (curl-pinned externals: plugins.lock.json's +# managed_by:curl entries + link.sh's EXTERNAL_SKILLS array — the OTHER +# externals the GStack section above does not cover) +# ──────────────────────────────────────────────────────────── +echo "── Vendored skills ──" +# Mirrors lib/profile.sh's active_profile() (read_cache + the +# blank/"none" -> DEFAULT_PROFILE fallback) without sourcing profile.sh +# itself (its main() would run unconditionally) and without ever +# invoking `claude`. +_dv_active_profile=$(head -n1 "$REPO/.active-profile" 2>/dev/null \ + | tr -d '[:space:]') +[ -z "$_dv_active_profile" ] && _dv_active_profile="none" +[ "$_dv_active_profile" = "none" ] && _dv_active_profile="full" +# .active-profile's value is spliced into a lib/profiles/ path below — +# reject anything outside the profile-name allowlist before that splice. +if ! _dv_valid_profile_name "$_dv_active_profile"; then + warn ".active-profile has invalid value \"$_dv_active_profile\" — \ +falling back to profile full" + _dv_active_profile="full" +fi +_dv_profile_file="$REPO/lib/profiles/$_dv_active_profile.profile" +if [ -f "$_dv_profile_file" ]; then + check_vendored_skills "$REPO" "$HOME/.claude" "$_dv_profile_file" +else + # Active profile unresolved — every external is expected linked. + check_vendored_skills "$REPO" "$HOME/.claude" +fi +unset _dv_active_profile _dv_profile_file + +echo "" + # ── Playwright browsers (read-only report; NOT nested under gstack — 2 of # the 3 registered installs are gsd-pi, not gstack) ── echo "── Playwright browsers ──" diff --git a/lib/doctor-vendored.sh b/lib/doctor-vendored.sh new file mode 100644 index 0000000..6cd9602 --- /dev/null +++ b/lib/doctor-vendored.sh @@ -0,0 +1,257 @@ +#!/usr/bin/env bash +# ============================================================ +# lib/doctor-vendored.sh — doctor.sh check for the externally vendored +# skills (curl-pinned externals in plugins.lock.json + link.sh's +# EXTERNAL_SKILLS array). doctor.sh's "GStack submodule" section only +# covers the gstack submodule — this covers the OTHER external skill +# packs (emil-design-eng, the agent-skills trio, the five Mengto scroll +# skills, and any name link.sh links with no lock entry at all, e.g. +# frontend-design, design-motion-principles). +# +# One entry point, `check_vendored_skills +# [profile_file]`, sourced and called by doctor.sh. Two things checked +# per name in link.sh's EXTERNAL_SKILLS array: +# 1. its file(s) exist under skills-external// — expected file +# list comes from the matching plugins.lock.json entry (list shape +# -> ["SKILL.md"], dict shape -> its own file list, the +# emil-design-eng single-file "path" shape -> the key itself is the +# name, file "SKILL.md") or, when no lock entry names it at all, +# defaults to ["SKILL.md"]. +# 2. when the name is listed in (or no +# is passed — the "could not resolve the active profile" case), +# the /skills/ symlink points at +# /skills-external/. A name absent from the profile is +# reported parked, not failed. +# +# Lock parsing via python3 argv (never string-spliced) — same pattern as +# lib/vendor-skills.sh's _vendor_read_lock. link.sh's EXTERNAL_SKILLS +# array is parsed with a single-purpose grep/sed, tolerant to it +# spanning multiple lines. +# +# Every name/file pulled from the lock or link.sh is spliced into a +# filesystem path (skills-external//, +# /skills/): _dv_valid_item_name allowlists it first +# (a rejection is a warn + skip, never a fail). doctor.sh's active +# profile splices into lib/profiles/.profile the same way, guarded +# by _dv_valid_profile_name. +# +# No `set -euo pipefail` here (mirrors lib/vendor-skills.sh): a sourced +# lib must not change the caller's shell options. +# ============================================================ + +# Fallback color helpers when sourced standalone (e.g. the test suite) — +# skip anything the caller (doctor.sh) already defines, so doctor.sh's +# ERRORS/WARNS counters keep working. +if ! declare -F pass >/dev/null 2>&1; then + GREEN='\033[0;32m'; NC='\033[0m' + pass() { echo -e " ${GREEN}✓${NC} $1"; } +fi +if ! declare -F fail >/dev/null 2>&1; then + RED='\033[0;31m'; NC='\033[0m' + fail() { echo -e " ${RED}✗${NC} $1"; } +fi +if ! declare -F warn >/dev/null 2>&1; then + YELLOW='\033[1;33m'; NC='\033[0m' + warn() { echo -e " ${YELLOW}⚠${NC} $1"; } +fi +if ! declare -F info >/dev/null 2>&1; then + BLUE='\033[0;34m'; NC='\033[0m' + info() { echo -e " ${BLUE}→${NC} $1"; } +fi + +# _dv_lock_expectations — prints "\t" for every +# skill named under a plugins.lock.json entry whose "managed_by" is +# "curl": a bare list defaults each name to ["SKILL.md"]; a dict names +# its own per-skill file list; an entry with neither (the +# emil-design-eng single-file "path" shape) is itself the skill name, +# file "SKILL.md" (the literal "path" value is upstream layout, not the +# local dest — never used here). Reads the lockfile via argv only. +# Every curl-managed entry's shape is validated ("skills" null, a list +# of str, or a dict of str -> list of str; "path" a str when present) +# BEFORE it is used, so a malformed entry is the same clean failure as +# an unreadable file: rc 1, nothing printed. The python3 call's stderr +# is discarded — no traceback ever reaches the caller's terminal, only +# the rc reaches bash's decision. +_dv_lock_expectations() { + python3 - "$1" 2>/dev/null <<'PY' +import json, sys + + +def valid_skills(skills): + """True when "skills" is null, a list of str, or a dict of + str -> list of str — the only shapes this lock format allows.""" + if skills is None: + return True + if isinstance(skills, list): + return all(isinstance(name, str) for name in skills) + if isinstance(skills, dict): + return all( + isinstance(name, str) and isinstance(files, list) + and all(isinstance(f, str) for f in files) + for name, files in skills.items() + ) + return False + + +def skill_files(skills): + """Normalize an already-validated "skills" value to + {name: [file, ...]} — a bare list defaults to ["SKILL.md"].""" + if isinstance(skills, list): + return {name: ["SKILL.md"] for name in skills} + return skills + + +try: + with open(sys.argv[1]) as f: + data = json.load(f) +except (OSError, ValueError): + sys.exit(1) + +if not isinstance(data, dict): + sys.exit(1) + +for key, entry in data.items(): + if not isinstance(entry, dict) or entry.get("managed_by") != "curl": + continue + skills, path = entry.get("skills"), entry.get("path") + if path is not None and not isinstance(path, str): + sys.exit(1) + if not valid_skills(skills): + sys.exit(1) + if skills is None: + print(f"{key}\tSKILL.md") + continue + for name, files in skill_files(skills).items(): + for file in files: + print(f"{name}\t{file}") +PY +} + +# _dv_link_names — prints one name per line from link.sh's +# EXTERNAL_SKILLS=(...) array, tolerant to it spanning multiple lines. +# rc 1 (nothing printed) when the array marker is absent from the file. +_dv_link_names() { + local link_sh="$1" + grep -qF 'EXTERNAL_SKILLS=(' "$link_sh" 2>/dev/null || return 1 + awk '/EXTERNAL_SKILLS=\(/{f=1} f{print} f&&/\)/{exit}' "$link_sh" \ + | sed -e 's/^.*EXTERNAL_SKILLS=(//' -e 's/).*$//' \ + | tr -s '[:space:]' '\n' \ + | grep -v '^$' +} + +# _dv_profile_has — true when a line's FIRST +# whitespace-separated token equals (the profile line's label +# column — comments and the type column are ignored). +_dv_profile_has() { + local profile_file="$1" name="$2" + awk -v n="$name" '$1 == n { found=1 } END { exit !found }' "$profile_file" +} + +# _dv_valid_profile_name — true when matches the +# profile-name allowlist (letters, digits, underscore, hyphen only). +# is spliced into "lib/profiles/.profile" by doctor.sh, so +# a path-traversal or separator character must never reach it. +_dv_valid_profile_name() { + [[ "$1" =~ ^[A-Za-z0-9_-]+$ ]] +} + +# _dv_valid_item_name — true when (a skill name from +# link.sh's EXTERNAL_SKILLS array, or a relative file named by a +# plugins.lock.json entry) matches the item-name allowlist (letters, +# digits, dot, underscore, hyphen, slash), has no leading "/" and no +# ".." path segment. is spliced into a filesystem path under +# skills-external/ or /skills/. +_dv_valid_item_name() { + local name="$1" + [[ "$name" =~ ^[A-Za-z0-9._/-]+$ ]] || return 1 + case "$name" in /*) return 1 ;; esac + case "/$name/" in */../*) return 1 ;; esac +} + +# _dv_check_files — every file (the +# "\t" lines from _dv_lock_expectations) names for , +# defaulting to just "SKILL.md" when names it no file at all +# (a link.sh-only name with no lock entry). fail per missing file. Each +# is checked against the item-name allowlist before it is spliced +# into a path — a rejected one is warned and skipped, not failed. rc 0 +# only when every expected (and allowlisted) file is present. +_dv_check_files() { + local repo="$1" name="$2" lock_out="$3" + local files rel dest all_ok=1 + files="$(awk -F'\t' -v n="$name" '$1 == n { print $2 }' <<<"$lock_out")" + [ -n "$files" ] || files="SKILL.md" + while IFS= read -r rel; do + [ -n "$rel" ] || continue + if ! _dv_valid_item_name "$rel"; then + warn "$name: lock file entry \"$rel\" rejected by the item-name \ +allowlist — skipped" + continue + fi + dest="$repo/skills-external/$name/$rel" + if [ ! -f "$dest" ]; then + fail "$name: skills-external/$name/$rel missing — run: make plugin" + all_ok=0 + fi + done <<< "$files" + [ "$all_ok" -eq 1 ] +} + +# _dv_check_link — when +# is non-empty and does not list , reports it +# parked (info), not failed. Otherwise (listed, or no was +# passed — active profile could not be resolved, every external is then +# expected linked) checks the /skills/ symlink points +# at /skills-external/. +_dv_check_link() { + local claude_home="$1" repo="$2" name="$3" profile_file="$4" + local link target label + if [ -n "$profile_file" ] && ! _dv_profile_has "$profile_file" "$name"; then + label="$(basename "$profile_file" .profile)" + info "$name: parked by profile $label" + return + fi + link="$claude_home/skills/$name" + target="$repo/skills-external/$name" + if [ -L "$link" ] && [ "$(readlink "$link")" = "$target" ]; then + pass "$name: vendored + linked" + else + fail "$name: symlink missing/wrong — run: make link (or: bash \ +lib/profile.sh apply )" + fi +} + +# check_vendored_skills [profile_file] — see the +# file header. Either the lock or link.sh being unreadable (or a +# malformed lock entry — _dv_lock_expectations rc 1) is a warn, never a +# fail; link.sh unreadable skips the whole check (there is nothing to +# iterate). Each from link.sh is checked against the item-name +# allowlist before it is spliced into a path — a rejected one is +# warned and skipped, not failed. +check_vendored_skills() { + local repo="$1" claude_home="$2" profile_file="${3:-}" + local lock_out names name + + if ! lock_out="$(_dv_lock_expectations "$repo/plugins.lock.json")"; then + warn "plugins.lock.json unreadable or malformed (missing, invalid \ +JSON, or an entry with a bad \"skills\"/\"path\" shape) — \ +vendored-skills file check falls back to SKILL.md-only defaults" + lock_out="" + fi + + if ! names="$(_dv_link_names "$repo/link.sh")" || [ -z "$names" ]; then + warn "link.sh EXTERNAL_SKILLS array unreadable — vendored-skills \ +check skipped" + return 0 + fi + + while IFS= read -r name; do + [ -n "$name" ] || continue + if ! _dv_valid_item_name "$name"; then + warn "link.sh EXTERNAL_SKILLS entry \"$name\" rejected by the \ +item-name allowlist — skipped" + continue + fi + _dv_check_files "$repo" "$name" "$lock_out" \ + && _dv_check_link "$claude_home" "$repo" "$name" "$profile_file" + done <<< "$names" +} diff --git a/lib/tests/doctor-vendored.test.sh b/lib/tests/doctor-vendored.test.sh new file mode 100644 index 0000000..7ee2006 --- /dev/null +++ b/lib/tests/doctor-vendored.test.sh @@ -0,0 +1,216 @@ +#!/usr/bin/env bash +# lib/tests/doctor-vendored.test.sh — lib/doctor-vendored.sh's +# check_vendored_skills(): a fixture repo under mktemp with a fake +# plugins.lock.json (single-path shape, list shape, dict shape with a +# references/ file), a fake link.sh holding a multi-line +# EXTERNAL_SKILLS=(...) array, a fake /skills dir and a fake +# profile file. Cases: every file present + linked (ALL_PRESENT), a +# list-shape skill missing its SKILL.md (FILE_MISSING), a dict-shape +# skill missing one of two files — only that file is named +# (DICT_FILES_COMPLETE), a profile-listed name with no symlink +# (SYMLINK_MISSING_ACTIVE) or a symlink to the wrong target +# (SYMLINK_WRONG_TARGET), a name absent from the profile reported parked +# rather than failed (SYMLINK_PARKED), the same name treated as +# expected-linked (fail, not parked) when no profile file is passed at +# all (NO_PROFILE_EXPECTS_LINK), an unreadable lock file degrading to a +# warn instead of a fail (LOCK_UNREADABLE, rc 0), a lock entry whose +# "skills" is neither null/list/dict degrading the same way with no +# Python traceback leaking (LOCK_MALFORMED_ENTRY, rc 0), the +# profile-name allowlist rejecting a path-traversal value +# (REJECTS_BAD_PROFILE_NAME), and the item-name allowlist rejecting a +# link.sh entry with a ".." segment — warned and skipped, not failed +# (REJECTS_BAD_NAME). +set -u +ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +LIB="$ROOT/lib/doctor-vendored.sh" +pass=0; fail=0 + +check_bool() { + local name="$1" ok="$2" + if [ "$ok" = 1 ]; then pass=$((pass+1)); echo "PASS $name" + else fail=$((fail+1)); echo "FAIL $name"; fi +} + +WORK="$(mktemp -d)"; trap 'rm -rf "$WORK"' EXIT +REPO="$WORK/repo" +CLAUDE_HOME="$WORK/claude_home" +mkdir -p "$REPO/skills-external" "$CLAUDE_HOME/skills" + +# ── Lock: "ok-skill" mirrors the emil-design-eng single-file "path" +# shape (the key IS the name); "list-entry" mirrors the agent-skills +# bare-list shape (5 names, each defaults to SKILL.md); "dict-entry" +# mirrors the mengto-skills explicit shape (dict-skill needs SKILL.md + +# references/notes.md, and only the latter is ever missing below). +cat > "$REPO/plugins.lock.json" <<'JSON' +{ + "ok-skill": { + "path": "skills/ok-skill/SKILL.md", + "managed_by": "curl" + }, + "list-entry": { + "managed_by": "curl", + "skills": [ + "missing-skill", "active-nolink-skill", "active-wronglink-skill", + "parked-skill", "noprofile-skill" + ] + }, + "dict-entry": { + "managed_by": "curl", + "skills": {"dict-skill": ["SKILL.md", "references/notes.md"]} + } +} +JSON + +# ── link.sh: a multi-line EXTERNAL_SKILLS array, tolerance-tested. +cat > "$REPO/link.sh" <<'SH' +#!/usr/bin/env bash +EXTERNAL_SKILLS=(ok-skill missing-skill dict-skill + active-nolink-skill active-wronglink-skill + parked-skill noprofile-skill) +SH + +# ── skills-external/ tree: every name's SKILL.md present, except +# missing-skill (nothing at all) and dict-skill's references/notes.md. +for n in ok-skill dict-skill active-nolink-skill active-wronglink-skill \ + parked-skill noprofile-skill; do + mkdir -p "$REPO/skills-external/$n" + echo "v1" > "$REPO/skills-external/$n/SKILL.md" +done + +# ── claude_home symlinks: ok-skill correct, active-wronglink-skill +# points elsewhere, active-nolink-skill and noprofile-skill have none. +ln -sf "$REPO/skills-external/ok-skill" "$CLAUDE_HOME/skills/ok-skill" +mkdir -p "$WORK/elsewhere" +ln -sf "$WORK/elsewhere" "$CLAUDE_HOME/skills/active-wronglink-skill" + +# ── active.profile: lists everything EXCEPT parked-skill and +# noprofile-skill (both proven absent from it). +cat > "$REPO/active.profile" <<'PROF' +# DESC: fixture profile +ok-skill external +missing-skill external +dict-skill external +active-nolink-skill external +active-wronglink-skill external +PROF + +# shellcheck source=../doctor-vendored.sh disable=SC1091 +source "$LIB" + +# ── With the active profile passed ────────────────────────────────── +out1="$(check_vendored_skills "$REPO" "$CLAUDE_HOME" \ + "$REPO/active.profile" 2>&1)" + +check_bool ALL_PRESENT \ + "$(printf '%s' "$out1" | grep -qF 'ok-skill: vendored + linked' \ + && echo 1 || echo 0)" + +check_bool FILE_MISSING \ + "$(printf '%s' "$out1" | \ + grep -qF 'missing-skill: skills-external/missing-skill/SKILL.md missing' \ + && echo 1 || echo 0)" + +check_bool DICT_FILES_COMPLETE \ + "$(printf '%s' "$out1" | grep -qF \ + 'dict-skill: skills-external/dict-skill/references/notes.md missing' \ + && ! printf '%s' "$out1" | grep -qF \ + 'dict-skill: skills-external/dict-skill/SKILL.md missing' \ + && echo 1 || echo 0)" + +check_bool SYMLINK_MISSING_ACTIVE \ + "$(printf '%s' "$out1" | \ + grep -qF 'active-nolink-skill: symlink missing/wrong' && echo 1 || echo 0)" + +check_bool SYMLINK_WRONG_TARGET \ + "$(printf '%s' "$out1" | \ + grep -qF 'active-wronglink-skill: symlink missing/wrong' \ + && echo 1 || echo 0)" + +check_bool SYMLINK_PARKED \ + "$(printf '%s' "$out1" | grep -qF 'parked-skill: parked by profile active' \ + && ! printf '%s' "$out1" | \ + grep -qF 'parked-skill: symlink missing/wrong' \ + && echo 1 || echo 0)" + +# ── No profile file passed at all: noprofile-skill (absent from +# active.profile, parked above) must now be treated as expected-linked. +out2="$(check_vendored_skills "$REPO" "$CLAUDE_HOME" 2>&1)" +check_bool NO_PROFILE_EXPECTS_LINK \ + "$(printf '%s' "$out2" | \ + grep -qF 'noprofile-skill: symlink missing/wrong' && echo 1 || echo 0)" + +# ── LOCK_UNREADABLE — a second, minimal fixture with an invalid +# plugins.lock.json: check_vendored_skills degrades to a warn (not a +# fail) and still returns 0. +BROKEN="$WORK/broken" +mkdir -p "$BROKEN/skills-external/solo-skill" +echo "v1" > "$BROKEN/skills-external/solo-skill/SKILL.md" +echo "not valid json" > "$BROKEN/plugins.lock.json" +cat > "$BROKEN/link.sh" <<'SH' +#!/usr/bin/env bash +EXTERNAL_SKILLS=(solo-skill) +SH +out3="$(check_vendored_skills "$BROKEN" "$CLAUDE_HOME" 2>&1)" +rc3=$? +check_bool LOCK_UNREADABLE \ + "$([ "$rc3" -eq 0 ] && printf '%s' "$out3" | \ + grep -qF 'plugins.lock.json unreadable' && echo 1 || echo 0)" + +# ── LOCK_MALFORMED_ENTRY — a valid-JSON lock whose "skills" is a bare +# number (neither null, list nor dict): degrades to the same warn as an +# unreadable lock, rc 0, and no Python traceback text anywhere in the +# captured output. +MALFORMED="$WORK/malformed" +mkdir -p "$MALFORMED/skills-external/bad-entry" +echo "v1" > "$MALFORMED/skills-external/bad-entry/SKILL.md" +cat > "$MALFORMED/plugins.lock.json" <<'JSON' +{ + "bad-entry": { + "managed_by": "curl", + "skills": 42 + } +} +JSON +cat > "$MALFORMED/link.sh" <<'SH' +#!/usr/bin/env bash +EXTERNAL_SKILLS=(bad-entry) +SH +out4="$(check_vendored_skills "$MALFORMED" "$CLAUDE_HOME" 2>&1)" +rc4=$? +check_bool LOCK_MALFORMED_ENTRY \ + "$([ "$rc4" -eq 0 ] \ + && printf '%s' "$out4" | grep -qF 'plugins.lock.json unreadable' \ + && ! printf '%s' "$out4" | grep -qi 'traceback' \ + && ! printf '%s' "$out4" | grep -q 'Error:' \ + && echo 1 || echo 0)" + +# ── REJECTS_BAD_PROFILE_NAME — the profile-name allowlist helper +# rejects a path-traversal value and accepts a plain one. +check_bool REJECTS_BAD_PROFILE_NAME \ + "$( { _dv_valid_profile_name "full" \ + && ! _dv_valid_profile_name "../x" \ + && ! _dv_valid_profile_name "a/b" \ + && ! _dv_valid_profile_name "a.b"; } && echo 1 || echo 0)" + +# ── REJECTS_BAD_NAME — a link.sh EXTERNAL_SKILLS entry with a ".." +# segment: warned and skipped, never reaches _dv_check_files/ +# _dv_check_link (no fail line names it), rc 0. +BADNAME="$WORK/badname" +mkdir -p "$BADNAME/skills-external" +echo '{}' > "$BADNAME/plugins.lock.json" +cat > "$BADNAME/link.sh" <<'SH' +#!/usr/bin/env bash +EXTERNAL_SKILLS=(../evil) +SH +out5="$(check_vendored_skills "$BADNAME" "$CLAUDE_HOME" 2>&1)" +rc5=$? +check_bool REJECTS_BAD_NAME \ + "$([ "$rc5" -eq 0 ] \ + && printf '%s' "$out5" | \ + grep -qF '"../evil" rejected by the item-name allowlist' \ + && ! printf '%s' "$out5" | grep -qF 'skills-external/../evil' \ + && ! printf '%s' "$out5" | grep -qF '../evil: symlink' \ + && echo 1 || echo 0)" + +echo "PASS=$pass FAIL=$fail" +[ "$fail" -eq 0 ]