[diberry]

Summary

Extends security-review.mjs with Tier 1 pattern detection for skill markdown files (.copilot/skills/**/*.md and .squad/skills/**/*.md). Zero new dependencies, zero workflow changes, ~2-3s added scan time.

What's new

30 security patterns across 4 categories:

• P1: Embedded Credentials (10 patterns) — AWS keys, GitHub PAT/OAuth/App tokens, OpenAI keys, private keys, JWT, npm tokens, Slack tokens, generic secret assignments
• P2: Credential File Reads (7 patterns) — cat/type/readFileSync/Get-Content for .env, .ssh/id_rsa, .aws/credentials, .npmrc, .netrc, .pem/.key files (excludes .env.example/.env.sample/.env.template)
• P3: Download-and-Execute (8 patterns) — curl|bash, wget|bash, irm|iex, Invoke-Expression, powershell -enc, eval/source/bash process substitution
• P4: Privilege Escalation (5 patterns) — sudo bash/sh/rm, RunAs admin, Set-ExecutionPolicy Bypass/Unrestricted, chmod 777

Phase 1 Suppression (5 layers):

• Fenced code blocks (CommonMark-compliant: backtick/tilde, variable length, leading spaces)
• Inline code spans (backtick pairs) — prevents false positives on documentation examples
• Regex-doc table rows — suppresses credential matches on tables documenting regex patterns
• Placeholder tokens (sk-..., ghp_xxxx, AKIA..., angle brackets)
• Fail-safe: unclosed fences = unsuppressed (fail-open for security)

Architecture:

• Pure scanSkillContent(content, filePath) function exported for testing
• Main flow wrapped in run() with import.meta.url guard (importing for tests doesn't trigger git/console side effects)
• Existing 8 checks untouched; .copilot/skills/ skip for unsafe-git (check Feature: Issue Assignment & Auto-Triage for Squad Members #4) preserved
• New categories: skill-credentials, skill-credential-file-read, skill-download-exec, skill-privilege-escalation

Testing

110 new tests + 37 existing tests = 147 tests, all passing:

• Every pattern has positive + negative cases
• Fenced code block, inline code, regex-doc table, placeholder, and fail-open suppression tested
• Golden corpus: zero false positives across all 35 existing skill files

Not in scope (Phase 2)

• Context classifier (anti-pattern sections, heading-aware suppression)
• P5-P11 patterns
• Credentialed URL detection, frontmatter injection

Closes #881, supersedes #879

Working as Booster (CI/CD Engineer)

[github-actions]

🟡 Impact Analysis — PR #882

Risk tier: 🟡 MEDIUM

📊 Summary

Metric	Count
Files changed	4
Files added	2
Files modified	2
Files deleted	0
Modules touched	3

🎯 Risk Factors

• 4 files changed (≤5 → LOW)
• 3 modules touched (2-4 → MEDIUM)

📦 Modules Affected

root (1 file)

• .changeset/skill-security-scanner.md

scripts (1 file)

• scripts/security-review.mjs

tests (2 files)

• test/scripts/security-review-skills.test.ts
• test/scripts/security-review.test.ts

---

This report is generated automatically for every PR. See #733 for details.

[Copilot]

Pull request overview

Adds Phase 1 markdown-aware security scanning for skill files to the existing scripts/security-review.mjs repo-health scanner, with accompanying test coverage and a changeset.

Changes:

• Extend security-review.mjs to scan changed skill markdown files under .copilot/skills/** and .squad/skills/** for Tier 1 patterns (credentials, download-and-exec, privilege escalation).
• Export a pure scanSkillContent() function and guard script execution so it can be imported safely from tests.
• Add a dedicated Vitest suite for skill scanning behavior and register the new finding categories in the existing schema test.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
scripts/security-review.mjs	Adds markdown-aware scanning (scanSkillContent) and integrates it into the main security-review run.
test/scripts/security-review.test.ts	Extends the allowed category allowlist to include new skill-* categories.
test/scripts/security-review-skills.test.ts	New comprehensive unit tests for skill scanning patterns + suppression behavior + golden corpus.
.changeset/skill-security-scanner.md	Declares a patch release for the CLI package to ship the new scanner behavior.

[github-actions]

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 4076c6d

PR Scope: 🔧 Infrastructure

⚠️ 3 item(s) to address before review

Status	Check	Details
✅	Single commit	1 commit — clean history
✅	Not in draft	Ready for review
✅	Branch up to date	Up to date with dev
❌	Copilot review	No Copilot review yet — it may still be processing
✅	Changeset present	Changeset file found
✅	Scope clean	No .squad/ or docs/proposals/ files
✅	No merge conflicts	No merge conflicts
❌	Copilot threads resolved	2 unresolved Copilot thread(s) — fix and resolve before merging
❌	CI passing	1 check(s) still running
✅	Issue linked	Issue reference found
✅	Protected files	No protected bootstrap files changed

Files Changed (4 files, +866 −0)

File	+/−
.changeset/skill-security-scanner.md	+5 −0
scripts/security-review.mjs	+206 −0
test/scripts/security-review-skills.test.ts	+651 −0
test/scripts/security-review.test.ts	+4 −0

Total: +866 −0

---

This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

[diberry]

How This Works — Quick Guide for Reviewers

What changed: scripts/security-review.mjs now scans skill files (.copilot/skills/**/*.md and .squad/skills/**/*.md) for security patterns when they appear in a PR diff.

How it runs: Same VM, same job, same step as the existing security review in squad-repo-health.yml. No new runners, no new workflows. Adds ~3 seconds to the existing step.

What it catches (Phase 1 — high-confidence only):

• P1: Embedded credentials — real AWS keys, GitHub PATs, OpenAI keys, PEM blocks, JWTs, npm/Slack tokens
• P2: Credential file reads — instructions to cat .env, read ~/.ssh/id_rsa, etc.
• P3: Download-and-execute — curl | bash, irm | iex, powershell -enc, etc.
• P4: Privilege escalation — sudo bash, chmod 777, Set-ExecutionPolicy Bypass, etc.

What it does NOT catch (deferred to Phase 2):

• Unsafe git operations (git add ., --force push) — existing skip preserved because the diff parser can't distinguish "do X" from "never do X" in documentation
• Generic paths/IPs/URLs — too noisy for skill docs that legitimately reference localhost, C:\, etc.

Suppression (avoids false positives):

• Lines inside fenced code blocks (CommonMark-compliant, handles 4-backtick nesting)
• Inline code spans (backtick-wrapped)
• Regex documentation tables (pattern docs in secret-handling skill)
• Placeholder tokens (sk-..., ghp_xxxx, AKIA...)
• Fail-safe: unclosed fences = unsuppressed (fail-open for security)

Enforcement level: Informational only. Posts a PR comment if findings exist. Does NOT block merge or fail CI. This is a heads-up, not a gate — appropriate for Phase 1 where suppression is simple.

Test coverage: 110 new tests + golden corpus validation (zero false positives across all 35 existing skill files). Pure scanSkillContent() export enables direct unit testing without git side effects.

Cost: Zero additional Actions minutes. Zero new dependencies. ~3 seconds added to existing runner.

PRD: #881 | Supersedes: #879 | Inspired by: microsoft/waza ComplianceChecker patterns

[tamirdresher]

✅ Reviewed by Tamir + Squad.

Excellent security scanner — clean architecture with pure scanSkillContent() for testability. 30 patterns across 4 categories with 5-layer suppression is thorough. The fail-open on unclosed fences is the right security posture. 110 new tests + golden corpus validation (zero false positives on 35 existing skills) gives strong confidence.

Two minor Copilot notes (non-blocking):

• Unused statSync import in test file
• .squad/skills skip for unsafe-git already handled by the existing skip line

Phase 1 informational-only is the right approach. Ship it.