diff --git a/.github/workflows/README.md b/.github/workflows/README.md index aa92750b..e66b687f 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -285,6 +285,88 @@ The workflow will fail if: - Only one deployment runs at a time (group: "pages") - New deployments cancel in-progress ones +### 4. `security-scan.yml` - Skill Security Scan + +**Purpose**: Scans skills for security vulnerabilities using [cisco-ai-skill-scanner](https://github.com/cisco-ai-defense/skill-scanner) with LLM-powered analysis. Detects prompt injection, data exfiltration, social engineering, and other AI agent security risks. + +**Triggers**: +- **Pull requests** → Scans changed packs only (when `**/skills/**` or `**/mcps.json` change) +- **Manual dispatch** → Scan specific PR or all packs +- **Excludes**: Draft pull requests + +**Cost optimization**: +- **Path filters**: Only triggers when skill files or MCP configs change +- **Concurrency**: Cancels in-progress scans when new commits are pushed +- **Prerequisite gate**: Waits for `compliance-check` and `skill-linter` to pass before running — if either fails, the scan is skipped to save LLM tokens + +**What it checks**: +- YAML/manifest injection risks +- Command injection via untrusted inputs +- Supply chain risks (unpinned dependencies) +- Data exfiltration patterns +- Social engineering triggers +- Cross-skill overlap and coordinated behavior +- Missing metadata (license, provenance) + +**Behavior**: +- **MEDIUM or higher findings** → ❌ Workflow fails, blocks PR merge +- **LOW/INFO only** → ✅ Workflow passes +- Posts scan summary as PR comment with collapsible report per pack +- Uploads detailed reports as workflow artifacts (30-day retention) + +**How to run locally**: +```bash +# Install scanner +uv pip install --system 'cisco-ai-skill-scanner[google]' + +# Set credentials +export SKILL_SCANNER_LLM_API_KEY= +export SKILL_SCANNER_LLM_MODEL=gemini/gemini-2.5-pro # or openai/gpt-5 + +# Scan a single pack +skill-scanner scan-all rh-virt/skills \ + --recursive --use-behavioral --use-llm \ + --check-overlap --enable-meta \ + --fail-on-severity medium \ + --format markdown --detailed \ + --output security-report.md +``` + +**Manual workflow dispatch**: +1. Go to Actions → Skill Security Scan +2. Click "Run workflow" +3. Provide: + - **PR number** (required) — PR to post results to + - **Scan all packs** — `true` for full scan, `false` for changed packs only + +**Secrets required**: +- `SKILL_SCANNER_LLM_API_KEY` — API key for LLM provider +- `SKILL_SCANNER_LLM_MODEL` — Model identifier (e.g., `gemini/gemini-2.5-pro`) + +**Performance**: +- ~10-15 minutes per pack (depends on number of skills and LLM response time) +- Uses `uv` instead of `pip` for faster dependency installation with caching + +**Related files**: +- `scripts/detect-changed-packs.sh` - Detects packs with changed files in PRs +- Security reports uploaded as workflow artifacts + +### 5. `gemini-code-review.yml` - Gemini Code Review + +**Purpose**: Automated code review using Google Gemini, validating PRs against project rules (CLAUDE.md, SKILL_DESIGN_PRINCIPLES.md). + +**Triggers**: +- **Pull requests** (opened, synchronize, reopened) +- **Manual dispatch** with PR number + +**What it does**: +- Fetches PR diff and project rules +- Sends to Gemini for review +- Posts review as PR comment (updates existing comment on re-runs) + +**Secrets required**: +- `GEMINI_API_KEY` — Google Gemini API key + ## Adding New Workflows When adding new workflows: @@ -358,7 +440,5 @@ This README should be updated when: - New validation levels are introduced - Troubleshooting patterns emerge -**Last Updated**: 2026-02-27 -**Workflows Count**: 3 (skill-spec-report.yml, compliance-check.yml, deploy-pages.yml) - -**Note**: `validate-skills.yml` was removed 2026-02-27. Repository-specific design principle validation (Tier 2) will be handled by a separate workflow. +**Last Updated**: 2026-05-05 +**Workflows Count**: 5 (skill-spec-report.yml, compliance-check.yml, deploy-pages.yml, security-scan.yml, gemini-code-review.yml) diff --git a/.github/workflows/skill-security-scan.yml b/.github/workflows/skill-security-scan.yml new file mode 100644 index 00000000..a114d72c --- /dev/null +++ b/.github/workflows/skill-security-scan.yml @@ -0,0 +1,316 @@ +name: Skill Security Scan + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + paths: + - '**/skills/**' + - '**/mcps.json' + workflow_dispatch: + inputs: + pr_number: + description: 'PR number to review' + required: true + type: number + scan_all: + description: 'Scan all packs (true) or changed only (false)' + required: false + default: 'false' + type: choice + options: + - 'true' + - 'false' + +concurrency: + group: security-scan-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: true + +permissions: + contents: read + pull-requests: write + checks: read + +jobs: + security-scan: + if: github.event.pull_request.draft == false || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Resolve PR number + id: pr + run: | + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + echo "number=${{ inputs.pr_number }}" >> "$GITHUB_OUTPUT" + else + echo "number=${{ github.event.pull_request.number }}" >> "$GITHUB_OUTPUT" + fi + + - name: Wait for prerequisite checks + if: github.event_name == 'pull_request' + uses: actions/github-script@v7 + with: + script: | + const requiredChecks = ['compliance-check', 'skill-linter']; + const sha = context.payload.pull_request.head.sha; + const maxWait = 300000; // 5 minutes + const interval = 15000; // 15 seconds + let elapsed = 0; + + core.info(`Waiting for checks on ${sha}: ${requiredChecks.join(', ')}`); + + while (elapsed < maxWait) { + const { data: { check_runs } } = await github.rest.checks.listForRef({ + owner: context.repo.owner, + repo: context.repo.repo, + ref: sha, + }); + + const results = requiredChecks.map(name => { + const run = check_runs.find(r => r.name === name); + return { name, status: run?.status, conclusion: run?.conclusion }; + }); + + const allCompleted = results.every(r => r.status === 'completed'); + if (allCompleted) { + const failed = results.filter(r => r.conclusion !== 'success'); + if (failed.length > 0) { + const summary = failed.map(r => `${r.name}: ${r.conclusion}`).join(', '); + core.setFailed(`Prerequisite checks failed (${summary}) — skipping security scan to save tokens`); + return; + } + core.info('All prerequisite checks passed — proceeding with security scan'); + return; + } + + const pending = results.filter(r => r.status !== 'completed').map(r => r.name); + core.info(`Waiting for: ${pending.join(', ')} (${elapsed / 1000}s elapsed)`); + await new Promise(r => setTimeout(r, interval)); + elapsed += interval; + } + + core.setFailed('Timed out waiting for prerequisite checks — skipping security scan'); + + - name: Set up uv + uses: astral-sh/setup-uv@v7 + with: + enable-cache: true + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Install skill-scanner + run: uv pip install --system 'cisco-ai-skill-scanner[google]' + + - name: Detect changed packs + id: detect + run: | + if [ "${{ github.event_name }}" = "workflow_dispatch" ] && [ "${{ github.event.inputs.scan_all }}" = "true" ]; then + PACKS=$(find . -maxdepth 2 -name skills -type d | sed 's|^\./||;s|/skills$||' | sort) + else + PACKS=$(bash scripts/detect-changed-packs.sh || true) + fi + + PACK_COUNT=$(echo "$PACKS" | grep -c . || true) + + if [ -z "$PACKS" ] || [ "$PACK_COUNT" -eq 0 ]; then + echo "changed=false" >> $GITHUB_OUTPUT + echo "packs=" >> $GITHUB_OUTPUT + echo "pack_count=0" >> $GITHUB_OUTPUT + echo "No packs with changes detected — skipping security scan" + elif [ "$PACK_COUNT" -gt 1 ]; then + echo "changed=true" >> $GITHUB_OUTPUT + echo "packs<> $GITHUB_OUTPUT + echo "$PACKS" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + echo "pack_count=$PACK_COUNT" >> $GITHUB_OUTPUT + echo "::warning::Multiple packs detected ($PACK_COUNT): $PACKS" + else + echo "changed=true" >> $GITHUB_OUTPUT + echo "packs<> $GITHUB_OUTPUT + echo "$PACKS" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + echo "pack_count=1" >> $GITHUB_OUTPUT + echo "Changed pack detected: $PACKS" + fi + + - name: Reject multi-pack PRs + if: steps.detect.outputs.changed == 'true' && steps.detect.outputs.pack_count != '1' && github.event_name != 'workflow_dispatch' + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + MARKER="" + PR_NUMBER="${{ steps.pr.outputs.number }}" + PACKS="${{ steps.detect.outputs.packs }}" + + PACK_LIST="" + while IFS= read -r pack; do + [ -z "$pack" ] && continue + PACK_LIST="${PACK_LIST}\n- \`${pack}\`" + done <<< "$PACKS" + + { + echo "$MARKER" + echo "## ⚠️ Skill Security Scan — Skipped" + echo "" + echo "This PR contains changes in **multiple packs**:" + echo -e "$PACK_LIST" + echo "" + echo "To keep security scans fast and cost-effective, please limit each PR to **one pack only**." + echo "Split your changes into separate PRs (one per pack) and the security scan will run automatically." + } > /tmp/comment.md + + EXISTING_COMMENT_ID=$(gh api --paginate \ + "/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \ + --jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \ + | head -1) + + if [ -n "$EXISTING_COMMENT_ID" ]; then + gh api \ + --method PATCH \ + "/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \ + -f body="$(cat /tmp/comment.md)" + else + gh pr comment "$PR_NUMBER" \ + --repo "${{ github.repository }}" \ + --body-file /tmp/comment.md + fi + + echo "❌ Multiple packs detected — aborting security scan" + exit 1 + + - name: Check scanner credentials + if: steps.detect.outputs.changed == 'true' && steps.detect.outputs.pack_count == '1' + id: creds + env: + SKILL_SCANNER_LLM_API_KEY: ${{ secrets.SKILL_SCANNER_LLM_API_KEY }} + run: | + if [ -z "$SKILL_SCANNER_LLM_API_KEY" ]; then + echo "::warning::SKILL_SCANNER_LLM_API_KEY not available (expected for fork PRs). Skipping security scan." + echo "available=false" >> "$GITHUB_OUTPUT" + else + echo "available=true" >> "$GITHUB_OUTPUT" + fi + + - name: Run security scan + if: steps.detect.outputs.pack_count == '1' && steps.creds.outputs.available == 'true' + id: scan + env: + SKILL_SCANNER_LLM_API_KEY: ${{ secrets.SKILL_SCANNER_LLM_API_KEY }} + SKILL_SCANNER_LLM_MODEL: ${{ secrets.SKILL_SCANNER_LLM_MODEL }} + run: | + mkdir -p security-reports + SCAN_FAILED=false + + while IFS= read -r pack; do + [ -z "$pack" ] && continue + echo "=== Scanning: $pack ===" + + skill-scanner scan-all "$pack/skills" \ + --recursive \ + --use-behavioral \ + --use-llm \ + --check-overlap \ + --enable-meta \ + --fail-on-severity medium \ + --format markdown \ + --detailed \ + --output "security-reports/security-report-${pack}.md" || SCAN_FAILED=true + + echo "" + done <<< "${{ steps.detect.outputs.packs }}" + + if [ "$SCAN_FAILED" = "true" ]; then + echo "scan_result=failed" >> $GITHUB_OUTPUT + else + echo "scan_result=passed" >> $GITHUB_OUTPUT + fi + + - name: Upload security reports + id: upload + if: steps.detect.outputs.pack_count == '1' && steps.creds.outputs.available == 'true' && always() + uses: actions/upload-artifact@v4 + with: + name: security-reports + path: security-reports/ + retention-days: 30 + if-no-files-found: ignore + + - name: Post scan summary + if: steps.detect.outputs.pack_count == '1' && steps.creds.outputs.available == 'true' && always() + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + MARKER="" + SCAN_RESULT="${{ steps.scan.outputs.scan_result }}" + ARTIFACT_URL="${{ steps.upload.outputs.artifact-url }}" + PR_NUMBER="${{ steps.pr.outputs.number }}" + + if [ "$SCAN_RESULT" = "passed" ]; then + ICON="✅" + else + ICON="❌" + fi + + { + echo "$MARKER" + echo "## $ICON Skill Security Scan" + echo "" + + if [ -d "security-reports" ]; then + for report in security-reports/security-report-*.md; do + [ -f "$report" ] || continue + pack=$(basename "$report" .md | sed 's/^security-report-//') + echo "
" + echo "📋 $pack" + echo "" + cat "$report" + echo "" + echo "
" + echo "" + done + else + echo "No reports generated." + fi + + echo "" + if [ -n "$ARTIFACT_URL" ]; then + echo "> 📦 [Download security reports]($ARTIFACT_URL) | [Workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})" + else + echo "> [Workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})" + fi + } > /tmp/comment.md + + EXISTING_COMMENT_ID=$(gh api --paginate \ + "/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \ + --jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \ + | head -1) + + if [ -n "$EXISTING_COMMENT_ID" ]; then + gh api \ + --method PATCH \ + "/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \ + -f body="$(cat /tmp/comment.md)" + else + gh pr comment "$PR_NUMBER" \ + --repo "${{ github.repository }}" \ + --body-file /tmp/comment.md + fi + + - name: Check scan results + if: steps.detect.outputs.pack_count == '1' && steps.creds.outputs.available == 'true' + run: | + if [ "${{ steps.scan.outputs.scan_result }}" = "failed" ]; then + echo "❌ Security scan found MEDIUM or higher severity issues — blocking merge" + exit 1 + else + echo "✅ Security scan passed — no MEDIUM or higher severity issues found" + fi diff --git a/scripts/detect-changed-packs.sh b/scripts/detect-changed-packs.sh new file mode 100755 index 00000000..1e539749 --- /dev/null +++ b/scripts/detect-changed-packs.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Detect packs with changed files in CI (PRs and pushes to main) +# Outputs unique pack names (one per line) for security scanning +# Exits 0 if no packs changed +# +# Detects changes in ANY file within a pack directory (skills, mcps.json, docs, etc.) + +set -e + +if [ -n "$VALIDATE_INCLUDE_UNCOMMITTED" ]; then + DIFF_CMD="git diff --name-only HEAD" +elif [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then + BASE_REF="${GITHUB_BASE_REF:-main}" + git fetch origin "$BASE_REF" 2>/dev/null || true + DIFF_CMD="git diff --name-only origin/$BASE_REF...HEAD" +elif [ "$GITHUB_EVENT_NAME" = "push" ]; then + BEFORE="${GITHUB_EVENT_BEFORE:-}" + if [ -z "$BEFORE" ] || [ "$BEFORE" = "0000000000000000000000000000000000000000" ]; then + exit 0 + fi + DIFF_CMD="git diff --name-only $BEFORE HEAD" +else + DIFF_CMD="git diff --name-only HEAD" +fi + +CHANGED_FILES=$($DIFF_CMD 2>/dev/null || true) + +if [ -z "$CHANGED_FILES" ]; then + exit 0 +fi + +# Extract pack names: first path component of files that live inside a pack with a skills/ directory +# Exclude dotfiles (.github, .claude, etc.) +echo "$CHANGED_FILES" \ + | grep -v '^\.' \ + | cut -d'/' -f1 \ + | sort -u \ + | while read -r dir; do + if [ -d "$dir/skills" ]; then + echo "$dir" + fi + done