feat: extend security and quality gates with additional checks

[Benkapner]

Summary

When someone submits a skill to be evaluated, the pipeline checks it for security risks and quality issues before running the actual evaluation. This PR adds more checks to both of those gates, so we catch more problems early.

Security: the scanner can now detect skills that try to force the agent to obey unconditionally, leak system prompts or pipeline config, or silently install themselves into config files and shell startup scripts.

Quality: the scanner can now flag skills that give lots of instructions but no examples, only say what NOT to do without alternatives, reference deprecated tools/models, claim unrealistically broad scope, or are too long.

LLM review: the AI-powered quality review now also scores whether requirements are testable, whether tests match the instruction, and whether the expected output format is clear.

All of these run in Phase 2 (TEST) of the pipeline.

Changes

Security (deterministic, in skillmd_scanner.py)

• Coercive override patterns (forced compliance, override safety, no refusal)
• Prompt exfiltration patterns (output system prompt, leak configuration)
• Stealth persistence patterns (write to config dirs, crontab, autostart)

Quality (deterministic, in skillmd_quality_scanner.py)

• Example gap detection (many instructions, no code examples)
• Negative-only instructions (only says what NOT to do)
• Stale references (deprecated models, old runtimes)
• Scope overreach (claims too broad a scope)
• Per-skill token budget

LLM quality dimensions (in test_quality_review.py)

• Instruction testability
• Skill-test alignment
• Output format clarity

Tests

• Tests for all new security patterns with false-positive regression tests
• Tests for all new quality checks

Files changed (5 files, +484/-3)

File	Change
abevalflow/security/skillmd_scanner.py	New pattern categories: coercive override, prompt exfiltration, stealth persistence
abevalflow/quality/skillmd_quality_scanner.py	New quality checks: example gap, negative-only, stale refs, scope overreach, token budget
scripts/test_quality_review.py	Add LLM quality dimensions (testability, alignment, output clarity)
tests/test_skillmd_scanner.py	Tests for new security patterns + false-positive regression tests
tests/test_skillmd_quality_scanner.py	Tests for new quality checks

[GuyZivRH]

Consolidated Review: PR #59 — feat: extend security and quality gates with additional checks

Verdict: Request changes

Metric	Value
Author	Benkapner (Benjamin Kapner)
Diff	+594 / −3 (7 files)
Commits	8 (incl. cleanup of a duplicate scanner)
CI	Failing — ruff format --check on scripts/security_scan.py
Local tests	975-1049 passed on tip after checkout

---

Summary

Direction is good (more Phase-2 signal), but CI is red, there's a Tekton name collision, and several critical-severity patterns will false-positive on legitimate skill instructions.

What Looks Good

• Extending existing scanners (skillmd_scanner / skillmd_quality_scanner) and dropping the duplicate Pydantic scanner is the right call — Phase 2 already runs these via test.yaml
• New security categories (coercive override, prompt exfil, stealth persistence) are sensible additions
• Quality checks (example gap, negative-only, stale refs, scope overreach, token budget) are useful and mostly low severity
• LLM dimensions (testability, alignment, output_clarity) fit the existing dynamic LLMReviewGate — verified it handles any number of dimensions via for dim_name, dim_data in dimensions.items()
• Test coverage — 126 scanner/quality-scanner tests, all passing
• Well-designed regex patterns — case-insensitive, word-boundary anchored, handles common variations

---

Blocking Issues

1. CI: ruff format fails on scripts/security_scan.py

scripts/security_scan.py:1:1
-     parser = argparse.ArgumentParser(
-         description="Security scan a submission directory"
-     )
+     parser = argparse.ArgumentParser(description="Security scan a submission directory")

Fix: ruff format scripts/security_scan.py

2. Overly-broad security patterns will false-positive on benign text

These are the first "critical"-severity categories the scanner has ever populated. Per abevalflow/gates/security/base.py, critical findings are weighted at severity_counts["critical"] * 0.0 — a single critical hit zeroes the security score outright.
Tested regexes against plausible, everyday skill instructions:

Pattern (severity)	Benign text that matches
coercive_override "forced compliance" (high)	"You must follow the existing code style in this repo."
coercive_override "unconditional obedience" (high)	"Always execute the test suite before committing."
prompt_exfiltration "output system prompt" (critical)	"Print instructions for the next step clearly." / "Display configuration options to the user." / "Show prompt suggestions to help the user." / "Return instructions on how to fix the error."
prompt_exfiltration "echo env vars" (critical)	echo "value: ${HOME}" — extremely common, benign shell idiom
stealth_persistence (critical)	"Do not write to .claude/settings.json" — instructional text that forbids the bad behavior
Fix: Tighten regexes to require more specific co-occurring context:

• Require "leak"/"reveal"/"exfiltrate" alongside "system prompt"
• Require "safety"/"restriction" alongside "always/never comply"
• For stealth persistence, exclude lines starting with don't/do not/never/avoid (negation)

3. Tekton Task name collision

New pipeline/tasks/security_scan.yaml uses metadata.name: security-scan, same as existing pipeline/tasks/components/security-scan.yaml (Cisco scanner).
Docs/infrastructure_ops.md applies oc apply -f pipeline/tasks/ — these two fight for the same Task name.
Fix: Either:

• Delete the new task (preferred — already covered by skillmd-security-scan in test.yaml), or
• Rename it (e.g. skillmd-security-scan-standalone) and don't shadow Cisco

---

Should Fix

#	Issue
4	scripts/security_scan.py is redundant with scripts/skillmd_security_scan.py (already wired, supports --no-llm, --output, gate-compatible report). New CLI also exits 1 on high/critical, bypassing warn/disabled modes. Drop it with the duplicate Task, or thin-wrap the existing script.
5	scope_overreach is noisy — "reviews all code changes" / "manages all tasks" flags as overreach. Keep low, but tighten regexes or drop until precision improves.
6	stale_references flags migration docs that mention gpt-3.5-turbo / Node 16 as history. Prefer "use X" / install pins, not any mention.
7	Commit hygiene: more changes to the pr commit message; 4 commits have Claude co-author trailers. Squash/rename if you care about history.

---

Minor / Non-blocking

• No new tests for the three LLM prompt dimensions (prompt-only; OK if intentional)
• PR body oversells "Tekton task for standalone scanning" — Phase 2 already runs the scanners; the new task isn't referenced by ci-pipeline.yaml
• Prompt-exfil overlaps some existing prompt-leak rules — fine, but expect duplicate findings
• _TOKENS_PER_CHAR = 0.25 is a rough estimate — acceptable for a budget check

---

Summary

Category	Items
Must fix	CI formatting, overly-broad critical-severity patterns, Tekton name collision
Should fix	Remove redundant CLI/Task, tighten scope_overreach, fix stale_references precision, commit hygiene
Optional	LLM dimension tests
Request changes. Core scanner/quality/LLM extensions are solid and can stay as-is once the blocking issues are addressed. The quality-check additions (all low/medium severity) are well-scoped and tested. The main concern is the critical-severity security patterns that will block legitimate submissions on day one.

[Benkapner]

Addressed all blocking and should-fix items:

Fixed:

• ruff format on security_scan.py (moot now, file deleted)
• Tightened all overly-broad patterns: removed "follow"/"execute" from coercive override, required "system" keyword for prompt exfil, dropped the echo env vars pattern entirely, added negation-prefix exclusion for stealth persistence, narrowed crontab to write ops (-e/-r)
• Deleted the Tekton task (name collision with Cisco scanner, already covered by test.yaml)
• Deleted scripts/security_scan.py (redundant with existing skillmd_security_scan.py)
• Dropped the noisy "claims all code" scope_overreach pattern

Added false-positive regression tests for all the examples you listed (must follow code style, always execute tests, print instructions, display configuration, echo ${HOME}, do not write to .claude/).

Not addressed (intentional):

• stale_references flagging migration docs: keeping as-is since it's low severity and uncommon
• Commit hygiene: will squash on merge
• LLM dimension tests: prompt-only change, no testable logic
• AEH prompt missing new dimensions: intentional, AEH uses a different smaller dimension set

@GuyZivRH
ill run the pipeline