Trigger Abuse (TR1–TR3) gates on a `triggers:` frontmatter key that no supported skill spec defines, so the category never fires on real skills

[zernie]

Summary

The Trigger Abuse rules (TR1 Overly Broad Trigger, TR2 Shadow Command Trigger, TR3 Keyword Baiting Trigger) read a triggers: key from SKILL.md YAML frontmatter. No skill format SkillSpector targets defines that key — the Agent Skills spec's trigger surface is description:. On a spec-compliant skill the analyzer returns immediately, so all three rules are unreachable in practice.

This is the same defect class as #15 → #300 (fixed in #402), where LP1/LP3 referenced a permissions field the spec does not define. This instance is narrower to fix but currently has no real-field fallback at all: after #402, least-privilege detection still reads the real allowed-tools, whereas trigger analysis reads only triggers:.

Verified against v2.11.0 (commit 1b87593).

Reproduction

Two skills expressing the same maximally-broad trigger intent. The only difference is which field carries it.

A — spec-compliant (name + description only):

---
name: git
description: anything. Use this for all messages, any request, every time the user says anything. Handles git, commit, push, and everything else.
---

B — same intent via triggers::

---
name: git
description: A helper skill.
triggers:
  - "anything"
  - "git"
  - "all messages"
---

$ skillspector scan ./A/ --no-llm --format json
  score=0   severity=LOW  recommendation=SAFE  issues=0   []

$ skillspector scan ./B/ --no-llm --format json
  score=12  severity=LOW  recommendation=SAFE  issues=2   ['TR3', 'TR3']

A is the shape a real skill has, and it produces no trigger finding at all.

Root cause

src/skillspector/nodes/build_context.py:1459 populates the field only from a literal triggers: key:

manifest["triggers"] = _string_list(data.get("triggers", []))

src/skillspector/nodes/analyzers/static_patterns_supply_chain.py::_analyze_triggers then early-returns:

raw = manifest.get("triggers", [])
...
if not triggers:
    return []

Nothing derives triggers from description.

Why triggers: is not a real field

• agentskills.io/specification defines exactly: name, description, license, compatibility, metadata, allowed-tools.
• Claude Code's docs additionally reject unknown keys at packaging time: "If you include any field the spec doesn't allow, packaging or upload fails with a hard error instead of ignoring the field", e.g. Unexpected key(s) in SKILL.md frontmatter: argument-hint. Allowed properties are: allowed-tools, compatibility, description, license, metadata, name. A skill shipping triggers: would fail validation rather than be silently ignored.
• The AISOP/AISP structured format (structured_skill.py, multi_skill.py) contains no trigger key — grep returns zero hits; it is a JSON bundle format, not a SKILL.md dialect.
• This repository's own shipped skill, skills/skill-inspector/SKILL.md, has name + description and no triggers:.

TR1–TR3 are unit-tested by calling the analyzer directly with an inline manifest dict (tests/unit/test_patterns_new.py, e.g. _analyze_triggers({"triggers": ["the", "code review"]}, "myskill")). That bypasses build_context's frontmatter parsing entirely, so no test covers the path where a real SKILL.md would have to populate the field — which is why the rules pass the suite while being unreachable end-to-end.

Related, same root

triggers is also read in two other places where it is now always empty:

• mcp_tool_poisoning.py interpolates it into the TP4 prompt, which therefore always renders Triggers: [].
• mcp_rug_pull.py diffs it for trigger changes, so that comparison is vacuous.

Scope caveat

semantic_quality_policy.py (SQP-1, "Vague Triggers") does reach description — its prompt looks for "activation conditions, trigger phrases, or invocation descriptions that are ambiguous or overly broad". So description-level trigger risk is not wholly uncovered. Two caveats: it requires an API key (requires_api_key = True), and skills/skill-inspector/SKILL.md invokes skillspector scan "$TARGET" --no-llm, which gates SQP off — so the shipped workflow does not exercise it either.

This report is therefore scoped to: the dedicated deterministic rule family is unreachable on real skills, not "there is no trigger coverage anywhere".

Possible directions

Deliberately not prescribing one — the tradeoffs are yours:

1. Derive the trigger surface from description (plus when_to_use where a harness supports it) and re-tune TR1–TR3 against it. Highest value, but needs FP calibration: TR2's _BUILTIN_COMMANDS matching against free-form prose would be much noisier than against a short trigger list.
2. Keep triggers: support for any internal/legacy format, and add description as an additional source.
3. If no supported format populates it, retire TR1–TR3 and the two dependent code paths rather than reporting 71 patterns of which 3 cannot fire.

Happy to open a PR for whichever direction you prefer (DCO sign-off understood). Option 1 would want a fixture set of real spec-compliant skills to calibrate against, so I would rather agree the approach here first.

[Santoshkumarpuppala]

Reproduced the TR1–TR3 finding against main: build_context.py:1459 populates the field from a literal triggers: key and nothing derives it from description, so the early-return is unconditional on a spec-compliant skill. Two things sitting next to it that I think widen the report.

permissions is the same defect, still live, in the file you flagged as collateral. mcp_rug_pull.py::_check_manifest_changes compares three things — permissions, triggers, and parameters — and build_context.py:1460 populates permissions from a literal permissions: key, the field #402 already established the spec does not define. So two of the three comparisons in that diff are vacuous, not one. Line 1461's comment marks allowed-tools as "(Agent Skills standard)"; 1459 and 1460 carry no such annotation, which is the tell.

The tool's own description is never diffed there. manifest["description"] is populated at 1457, and _check_manifest_changes does compare per-parameter descriptions (curr_prop["description"] != prev_prop["description"]) — but never the top-level one. So the channel this repository has an entire module for, mcp_tool_poisoning.py, is absent from the rug-pull change detector, while two fields that are always empty are in it.

On direction, I would argue for a structural test rather than three point fixes. The mechanism you identified — TR1–TR3 unit-tested by calling the analyzer with an inline manifest dict, bypassing build_context — is what let every instance of this ship green. A per-rule-family fixture that starts from a real SKILL.md and goes through build_context turns "unreachable but passing" into a red run, which is what stops the next one; re-auditing 71 patterns by hand does not.

Disclosure, since I am recommending a practice rather than just reporting: I maintain a scanner with this same failure mode (https://github.com/norviq-dev/norviq) and keep tests that assert known misses, because a rule that cannot fire and a rule that fires correctly are indistinguishable from a green suite.