Four robustness defects in `per-type-validation.py`

[joshcollins268]

Repo: legioncodeinc/vibe-coding-tools
File: .cursor/skills/queen-bee-stinger/references/scripts/per-type-validation.py
Commit tested: eecb4acfc30500a5e88a2828471215a6cc5b1ef4 (eecb4ac)
SHA-256 of file tested: 010489135a877ffa14fe95a1e41c906d4338a80e4b940b82b1dd2238c1b078e7
Environment: Python 3.13 / macOS 15 (Darwin 25.6.0), PyYAML 6.0.3
Reported by: Main Street Mentor — Josh Collins josh.collins@yourmainstreetmentor.com

---

How these were found, and why you're getting them

Main Street Mentor vendors this file byte-identical into msm-skills and wraps it in a CI gate
(_tooling/check-skills.py) that validates every skill's frontmatter on each PR. Attribution and
licence are retained per §3.1 — see the repo's _tooling/skill-validator/NOTICE.md.

A CodeRabbit review of the PR that added that gate
(Main-Street-Mentor/msm-skills#81)
flagged these four in your file, all as 🟠 Major. Credit for finding them is CodeRabbit's; we
reproduced each one against eecb4ac before writing this up, and every transcript below is real
output, not a reconstruction.

We deliberately have not patched our vendored copy — keeping it byte-identical is what makes the
provenance checkable and keeps our next sync from you a clean re-copy instead of a manual merge. So
these are going to you rather than being forked away quietly. We've hardened our own wrapper to
refuse malformed validator output instead, which covers defects 3 and 4 (they crash, and a
crash can be detected from outside). It cannot cover 1 and 2, because those produce a
perfectly well-formed report that is simply wrong — those need a fix in this file.

Two clusters, in priority order.

---

Cluster A — silent false negatives (a wrapper cannot detect these)

These are the dangerous ones. The validator exits 0, emits a clean JSON report, and reports
zero findings on input that is genuinely invalid. Anyone using this as a CI gate gets a green
check that means nothing. No amount of output validation on the caller's side can see a check that
was never run.

1. A YAML parse failure is silently downgraded to the fallback parser

Location: parse_frontmatter(), ~L204–213

if HAVE_YAML:
    try:
        data = yaml.safe_load(fm_text)
        return data if isinstance(data, dict) else {}
    except Exception:
        return parse_simple_yaml(fm_text)

When yaml.safe_load() raises, the bare except Exception falls through to parse_simple_yaml(),
whose docstring is explicit that it does not support nested maps, multi-line scalars, anchors, or
tags. It does not fail on syntax it cannot represent — it guesses. So malformed YAML becomes a
plausible-looking dict and the parse error is never reported.

parse_simple_yaml() is documented as the fallback for "when PyYAML is unavailable". Using it as
an error handler when PyYAML is available is a different thing, and it converts a hard parse
error into a silent misread.

Reproduce

$ cat d1/SKILL.md
---
name: d1
description: [unterminated
---
body

$ python3 per-type-validation.py --type skill --harness cowork --json d1
{
  "exit_code": 0,
  "findings": [],
  "path": "d1",
  "summary": { "errors": 0, "infos": 0, "warnings": 0 }
}
$ echo $?
0

yaml.safe_load raises on that frontmatter. The fallback sees rest.startswith("[") but not
endswith("]"), so it stores the literal string "[unterminated" — a 13-character description that
passes every length check. The file is not loadable by any harness that uses a real YAML parser, and
the validator calls it clean.

Suggested fix: report the parse error as an ERROR finding. Keep parse_simple_yaml() for the
genuine no-PyYAML case, and when it is the parser, reject syntax outside its documented subset
rather than guessing at it.

---

2. Non-string name / description skip every check silently

Location: validate_skill(), ~L266–296

Each check is guarded by isinstance(..., str):

elif isinstance(description, str):        # L273 — both Cowork length checks
if isinstance(name, str) and not is_placeholder(name) and "cowork" in harnesses:   # L283
if isinstance(name, str) and not is_placeholder(name) and "cursor" in harnesses:   # L293

With PyYAML present, name: [x] parses to a list, every guard is False, and all name
validation — length, kebab-case, reserved words, Cursor folder-name match — plus both description
length ceilings are skipped without a word. The guards are correct as crash-prevention; the problem
is that there is no else.

Reproduce — the same semantic content, differing only by brackets:

$ cat d2/SKILL.md
---
name: [Claude_BAD_NAME_THAT_IS_NOT_KEBAB]
description: [x]
---

$ python3 per-type-validation.py --type skill --harness cowork --json d2
{ "exit_code": 0, "findings": [], "summary": { "errors": 0, "infos": 0, "warnings": 0 } }

$ cat d2b/SKILL.md          # identical values, unbracketed
---
name: Claude_BAD_NAME_THAT_IS_NOT_KEBAB
description: x
---

$ python3 per-type-validation.py --type skill --harness cowork --json d2b
{
  "exit_code": 1,
  "findings": [
    { "severity": "ERROR", "label": "SKILL.md",
      "message": "name 'Claude_BAD_NAME_THAT_IS_NOT_KEBAB' is not kebab-case" },
    { "severity": "ERROR", "label": "SKILL.md",
      "message": "name 'Claude_BAD_NAME_THAT_IS_NOT_KEBAB' contains reserved word 'claude'" }
  ],
  "summary": { "errors": 2, "infos": 0, "warnings": 0 }
}

Two ERRORs and exit 1 become zero findings and exit 0, from adding brackets.

Suggested fix: before the length/format/reserved-word checks, emit an ERROR when name or
description is present but not a string. That closes the false negative and, as a bonus, removes
the crash risk if one of those isinstance guards is ever dropped during a refactor.

---

Cluster B — unhandled exceptions escape main()

main() catches only OSError (~L633). Any TypeError / AttributeError from a validator function
therefore propagates out, so the process dies with a traceback, prints nothing on stdout, and
exits 1 — which is also the validator's normal "found ERROR findings" exit code. Under --json
that means a caller distinguishing "clean" from "found errors" by exit code alone sees a normal
result for a run that never happened.

3. --type agent: list-valued color / memory / isolation raise TypeError

Location: validate_agent(), L372 / L376 / L380

if color is not None and color not in AGENT_COLOR_ENUM:

AGENT_COLOR_ENUM is a set, so an unhashable value raises before the membership test resolves.

Reproduce

$ cat d3/d3.md
---
name: d3
description: x
color: [red]
---

$ python3 per-type-validation.py --type agent --harness claude-code --json d3/d3.md
Traceback (most recent call last):
  File "per-type-validation.py", line 648, in <module>
    sys.exit(main())
  File "per-type-validation.py", line 624, in main
    validate_agent(root, harnesses, args.target, report)
  File "per-type-validation.py", line 372, in validate_agent
    if color is not None and color not in AGENT_COLOR_ENUM:
TypeError: cannot use 'list' as a set element (unhashable type: 'list')

$ echo $?
1
$ python3 per-type-validation.py --type agent --harness claude-code --json d3/d3.md 2>/dev/null | wc -c
0          # exit 1 with EMPTY stdout under --json

The same section has a false negative alongside the crash: a non-string name skips the agent name
checks entirely, same shape as defect 2.

Suggested fix: reject non-string color / memory / isolation / name with an ERROR before
the enum and regex tests. Independently, widening main()'s except OSError to also catch
Exception and emit a structured error report would make every remaining case of this class
survivable for JSON consumers.

---

4. --type plugin: a non-object manifest, or a non-string name, raises

Location: validate_plugin(), ~L490–501

data = json.loads(text)      # not checked for being a dict
...
name = data.get("name")      # L495
...
elif not PLUGIN_NAME_RE.match(name):    # L500

Reproduce

$ echo '[]' > d4a/.claude-plugin/plugin.json
$ python3 per-type-validation.py --type plugin --harness cowork --json d4a
  File "per-type-validation.py", line 495, in validate_plugin
    name = data.get("name")
AttributeError: 'list' object has no attribute 'get'
$ echo $?    ->  1, stdout empty

$ echo '{"name": 1}' > d4b/.claude-plugin/plugin.json
$ python3 per-type-validation.py --type plugin --harness cowork --json d4b
  File "per-type-validation.py", line 500, in validate_plugin
    elif not PLUGIN_NAME_RE.match(name):
TypeError: expected string or bytes-like object, got 'int'
$ echo $?    ->  1, stdout empty

Both are plausible authoring mistakes (a JSON array at the top level, a version-like value left
unquoted), and both are exactly the case a plugin validator exists to catch.

Suggested fix: after json.loads, ERROR and return if the result is not a dict; ERROR if
name is present but not a string, before the placeholder and regex checks.

---

Summary

#	Type	Location	Symptom	Detectable by a caller?
1	False negative	parse_frontmatter L204–213	Invalid YAML reported clean, exit 0	No
2	False negative	validate_skill L266–296	Non-string name/description skips all checks, exit 0	No
3	Crash	validate_agent L372/376/380	TypeError, empty stdout, exit 1	Yes
4	Crash	validate_plugin L495/500	AttributeError/TypeError, empty stdout, exit 1	Yes

The two clusters have different urgency. Cluster B is loud — anyone wrapping this script can detect
it from outside, and we do. Cluster A is silent, and a silent false negative in a validation gate is
worse than a crash: a crash stops the line, a false pass ships.

Happy to open a PR against vibe-coding-tools with fixes for all four if that's useful — say the
word and we'll shape it to your conventions rather than guess at them. The script is genuinely good
and it's doing real work for us; this is a bug report, not a complaint.