diff --git a/.github/workflows/skillgate.yml b/.github/workflows/skillgate.yml index 2176c19..719fde8 100644 --- a/.github/workflows/skillgate.yml +++ b/.github/workflows/skillgate.yml @@ -18,13 +18,17 @@ jobs: - uses: actions/checkout@v7 - uses: actions/setup-python@v6 with: - python-version: "3.11" + python-version-file: ".python-version" + - name: Install uv + uses: astral-sh/setup-uv@v6 + with: + enable-cache: true - name: Install SkillGate - run: python -m pip install -e ".[dev]" + run: uv sync --locked --group dev - name: Check golden snapshots id: snapshots continue-on-error: true - run: python tools/update_snapshots.py --check --artifacts test-outputs/snapshots + run: uv run python tools/update_snapshots.py --check --artifacts test-outputs/snapshots - name: Upload snapshot review artifacts if: steps.snapshots.outcome == 'failure' uses: actions/upload-artifact@v7 @@ -35,24 +39,24 @@ jobs: if: steps.snapshots.outcome == 'failure' run: exit 1 - name: Run tests - run: python -m pytest + run: uv run pytest - name: Run lint - run: python -m ruff check . + run: uv run ruff check . - name: Check formatting - run: python -m ruff format --check . + run: uv run ruff format --check . - name: Scan repository - run: skillgate scan . --format text + run: uv run skillgate scan . --format text - name: Run deterministic demo sessions run: | - skillgate demo skill --output test-outputs/reviewable-demo --validate --scan - skillgate demo mcpb --output test-outputs/reviewable-node.mcpb --scan + uv run skillgate demo skill --output test-outputs/reviewable-demo --validate --scan + uv run skillgate demo mcpb --output test-outputs/reviewable-node.mcpb --scan - name: Run policy smoke check - run: skillgate check fixtures/benchmark/01-safe-documentation-skill --policy skillgate.example.yaml + run: uv run skillgate check fixtures/benchmark/01-safe-documentation-skill --policy skillgate.example.yaml - name: Verify benchmark fixtures - run: skillgate fixtures summary fixtures/benchmark --format text + run: uv run skillgate fixtures summary fixtures/benchmark --format text - name: Generate SARIF if: always() - run: skillgate scan . --format sarif --output skillgate.sarif + run: uv run skillgate scan . --format sarif --output skillgate.sarif - name: Upload SARIF review artifact if: always() uses: actions/upload-artifact@v7 diff --git a/.python-version b/.python-version new file mode 100644 index 0000000..e4fba21 --- /dev/null +++ b/.python-version @@ -0,0 +1 @@ +3.12 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b34af0d..eaa5139 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -101,3 +101,20 @@ python tools/update_snapshots.py --accept ## Documentation Documentation should be clear about the threat model. SkillGate detects static risks and capability drift; it does not prove that an agent skill or MCP server is safe. + +# Development setup + +SkillGate uses Python 3.12 for its reproducible development environment. Install +[`uv`](https://docs.astral.sh/uv/) and run: + +```bash +uv sync --locked --group dev +uv run pytest +uv run ruff check . +uv run ruff format --check . +npm test +``` + +Use `uv run` for repository tools so local execution matches CI. If dependency +metadata changes, regenerate `uv.lock` with `uv lock`, review the diff, and +verify it with `uv sync --locked` before opening a pull request. diff --git a/README.md b/README.md index 7086837..5134eff 100644 --- a/README.md +++ b/README.md @@ -75,8 +75,30 @@ skillgate check . --policy skillgate.yaml ``` That is the primary workflow. Baselines, provenance, inventory reports, registry -comparison, SARIF upload, and advanced GitHub Action settings are available -after the first useful scan. +comparison, optional SARIF export, and optional GitHub Action settings are +available after the first useful scan. + +## Connection Boundaries + +The default path is local and upload-free: + +- `scan`, `skills validate`, `review preinstall` with a local path, and `mcpb + scan` read local files and write results to the terminal or local output files. +- These local paths do not call GitHub, upload reports, install packages, start + servers, or execute scanned content. + +Connections are explicit opt-ins: + +- `github scan` and `review preinstall` with a GitHub URL make bounded requests + to GitHub to fetch the requested source for static review. +- GitHub Actions, SARIF uploads, Actions artifacts, and Code Scanning are + optional integrations used only when a repository owner enables a workflow. + +Nothing in a local SkillGate invocation uploads findings automatically. + +For a copyable first repository, use the +[pre-install starter](examples/preinstall-starter/README.md) and its +[review-only Action workflow](docs/starter-repository.md). For concrete examples of how to interpret output, see the [public scan reports](docs/public-scan-reports/README.md). @@ -96,7 +118,7 @@ For a guided walkthrough with a deterministic local input, see the | Inspect an MCP bundle before installing | [`skillgate mcpb scan bundle.mcpb`](#5-scan-an-mcp-bundle-before-installing) | Static startup, file, endpoint, secret-reference, binary, and nested-archive review | | Inspect MCP registry metadata without installing | [`skillgate mcp registry scan`](#6-review-mcp-registry-metadata) | MCP tool, transport, package, and registry drift checks | | Build a review inventory | [`skillgate inventory .`](#7-build-a-capability-inventory) | Trust-boundary summary and filterable JSON | -| Upload findings to GitHub code scanning | [`--format sarif`](#8-export-sarif-for-github-code-scanning) | Stable SARIF alerts with capability tags | +| Optionally export SARIF for GitHub code scanning | [`--format sarif`](#8-optionally-export-sarif-for-github-code-scanning) | Local SARIF output; upload is a separate opt-in | | Follow a guided review session | [Review sessions](docs/sessions/README.md) | Copy-pasteable pre-install, pre-merge, and approval workflows | ## Install @@ -135,7 +157,8 @@ Teams that require maximum reproducibility should pin the full commit SHA in install commands and GitHub Action references. GitHub installs require Python 3.11 or newer and `git` on the customer machine. -After PyPI publication, the preferred low-friction install path will be: +PyPI publication is deferred for `0.1.3`. When it is intentionally revisited, +the planned distribution name is `openevalgate-skillgate`: ```bash pipx install openevalgate-skillgate @@ -162,9 +185,9 @@ is marked private until then. Details: [GitHub-first Node wrapper](docs/node-wra For contributor or source-checkout development: ```bash -python -m pip install -e . -skillgate --version -skillgate --help +uv sync --locked --group dev +uv run skillgate --version +uv run skillgate --help ``` ## 1. Scan A Local Repository @@ -346,9 +369,10 @@ skillgate inventory . --source-file "scripts/*" Inventory output groups detected capabilities and findings by source file and summarizes trust boundaries for local execution, remote endpoints, secrets, generated files, MCP servers, prompt controls, and obfuscation. -## 8. Export SARIF For GitHub Code Scanning +## 8. Optionally Export SARIF For GitHub Code Scanning -Any scan that supports SARIF can be uploaded to GitHub code scanning: +Any scan that supports SARIF can write a local SARIF file. This command does not +upload anything: ```bash skillgate scan . --format sarif --output skillgate.sarif @@ -360,7 +384,9 @@ skillgate mcp registry compare . --server io.example.server --format sarif --out Plain `scan` SARIF reports static findings. Policy-aware `check` SARIF includes policy waiver and suppression metadata while `--dry-run` keeps SARIF generation from becoming a second blocking step. SARIF output includes deterministic alert fingerprints, stable run categories, capability tags, severity tags, and capability taxa. Local scans use `skillgate/local-repository`, remote GitHub scans use `skillgate/remote-github`, MCPB scans use `skillgate/mcp-bundle`, and MCP registry comparisons use `skillgate/mcp-registry-compare`. -The repository includes `.github/workflows/skillgate.yml` as a complete example workflow and a composite action for CI adoption: +If you explicitly choose GitHub CI, the repository includes +`.github/workflows/skillgate.yml` and a composite Action example. Those workflows +are separate from local execution and may upload artifacts or SARIF to GitHub: ```yaml steps: @@ -376,7 +402,7 @@ steps: json-output: skillgate-review.json ``` -Add `policy: skillgate.yaml` when the repository has a policy file and should block unapproved behavior. Without `policy`, the Action runs a nonblocking scan. When both `policy` and `sarif-output` are supplied, the Action uploads policy-aware SARIF with waiver suppressions. +Add `policy: skillgate.yaml` when the repository has a policy file and should block unapproved behavior. Without `policy`, the optional Action runs a nonblocking scan. When both `policy` and `sarif-output` are supplied, that GitHub workflow can upload policy-aware SARIF with waiver suppressions. For pull-request review, add `step-summary: "true"` to publish a Markdown summary in the GitHub job page. Add `json-output: skillgate-review.json` when you also want a downloadable machine-readable review artifact with introduced capabilities, removed capabilities, changed trust boundaries, high-risk findings, policy violations, active waivers, and artifact locations. diff --git a/docs/benchmark/0.1.3.md b/docs/benchmark/0.1.3.md new file mode 100644 index 0000000..63e2aed --- /dev/null +++ b/docs/benchmark/0.1.3.md @@ -0,0 +1,71 @@ +# SkillGate Benchmark Report + +- Scanner version: `0.1.2` +- Fixture root: `fixtures/benchmark` +- Fixtures: 27 +- Passed: 27 +- Failed: 0 + +## Rule Coverage + +| Rule | Expected fixtures | Actual fixtures | Coverage status | +| --- | ---: | ---: | --- | +| SG001 | 7 | 7 | covered | +| SG002 | 2 | 2 | covered | +| SG003 | 19 | 19 | covered | +| SG004 | 2 | 2 | covered | +| SG005 | 6 | 6 | covered | +| SG006 | 5 | 5 | covered | +| SG007 | 3 | 3 | covered | +| SG008 | 1 | 1 | covered | +| SG009 | 9 | 9 | covered | +| SG010 | 1 | 1 | covered | +| SG011 | 2 | 2 | covered | +| SG012 | 2 | 2 | covered | + +## Fixture Results + +| Fixture | Status | Expected rules | Actual rules | Attribution | +| --- | --- | --- | --- | --- | +| `01-safe-documentation-skill` | **pass** | `none` | `none` | no | +| `02-shell-execution` | **pass** | `SG001` | `SG001` | no | +| `03-destructive-command` | **pass** | `SG001, SG002` | `SG001, SG002` | no | +| `04-network-egress` | **pass** | `SG003` | `SG003` | no | +| `05-remote-download-execute` | **pass** | `SG001, SG003, SG004` | `SG001, SG003, SG004` | no | +| `06-secret-access` | **pass** | `SG005` | `SG005` | no | +| `07-filesystem-write` | **pass** | `SG006` | `SG006` | no | +| `08-prompt-override` | **pass** | `SG007` | `SG007` | no | +| `09-unicode-obfuscation` | **pass** | `SG008` | `SG008` | no | +| `10-mcp-config` | **pass** | `SG003, SG005, SG009` | `SG003, SG005, SG009` | no | +| `11-mcp-capability-drift-before` | **pass** | `SG003, SG005, SG009` | `SG003, SG005, SG009` | no | +| `12-mcp-capability-drift-after` | **pass** | `SG001, SG003, SG005, SG009, SG010` | `SG001, SG003, SG005, SG009, SG010` | no | +| `13-public-pattern-python-node-extraction` | **pass** | `SG003, SG006` | `SG003, SG006` | yes | +| `14-public-pattern-shell-powershell-extraction` | **pass** | `SG001, SG002, SG003, SG006` | `SG001, SG002, SG003, SG006` | yes | +| `15-public-pattern-mcp-remote-config` | **pass** | `SG003, SG005, SG009` | `SG003, SG005, SG009` | yes | +| `16-public-pattern-mcp-http-remote` | **pass** | `SG003, SG009` | `SG003, SG009` | yes | +| `17-public-pattern-agent-skill-plugin` | **pass** | `SG003, SG009` | `SG003, SG009` | yes | +| `18-public-pattern-mcp-nested-profile` | **pass** | `SG003, SG005, SG009` | `SG003, SG005, SG009` | yes | +| `19-public-pattern-plugin-hooks` | **pass** | `SG001, SG003, SG004, SG006` | `SG001, SG003, SG004, SG006` | yes | +| `20-public-pattern-marketplace-mcp-package` | **pass** | `SG003, SG009` | `SG003, SG009` | yes | +| `21-public-pattern-agent-command-pack` | **pass** | `SG003, SG006` | `SG003, SG006` | yes | +| `22-public-pattern-mcp-local-bridge` | **pass** | `SG003, SG009` | `SG003, SG009` | yes | +| `23-public-pattern-skill-tool-metadata` | **pass** | `SG007` | `SG007` | yes | +| `24-public-pattern-mcp-tool-metadata-risk` | **pass** | `SG003, SG007, SG011` | `SG003, SG007, SG011` | yes | +| `25-public-pattern-mcp-app-web-surface` | **pass** | `SG003, SG011` | `SG003, SG011` | yes | +| `26-public-pattern-mcp-dangerous-transport` | **pass** | `SG001, SG003, SG012` | `SG001, SG003, SG012` | yes | +| `27-public-pattern-mcp-registry-package-metadata` | **pass** | `SG003, SG012` | `SG003, SG012` | yes | + +## Reproduce + +```bash +skillgate fixtures summary fixtures/benchmark --format markdown +``` + +## Attribution + +Public-pattern fixtures retain source URLs, retrieval dates, and reduction notes in their expected-findings metadata. + +## Limitations + +This report verifies deterministic fixture expectations and rule coverage. It is not a real-world detection accuracy benchmark, malware verdict, or completeness claim. +Fixtures are intentionally reduced examples and may not represent the behavior, context, or security posture of their source projects. diff --git a/docs/examples/github-action-minimal.md b/docs/examples/github-action-minimal.md index fea4c21..a75ad71 100644 --- a/docs/examples/github-action-minimal.md +++ b/docs/examples/github-action-minimal.md @@ -2,8 +2,22 @@ These examples use the stable `charliechenye/SkillGate@v0` Action tag. Teams that require immutable Action references should pin a full commit SHA instead. -SkillGate generates SARIF when `sarif-output` is supplied; GitHub's upload action -uploads that SARIF file to code scanning. +SkillGate generates SARIF when `sarif-output` is supplied. + +For pull requests, retain SARIF as an artifact so intentional demo and test +findings remain reviewable without creating a blocking Code Scanning status. +Publish SARIF to Code Scanning on protected branches or manual runs, or use the +blocking policy example below when the repository is ready to enforce reviewed +behavior. + +The copyable [pre-install starter repository](../../examples/preinstall-starter/) +contains this review-only workflow with Markdown, JSON, and SARIF artifacts. +On protected branches and manual runs, GitHub's upload action publishes the +SARIF artifact to Code Scanning and uploads that SARIF file for durable review. + +These Action examples are optional GitHub integrations. They do not change the +local-only behavior of SkillGate commands, which write reports locally and do +not upload findings. ## Nonblocking Scan With SARIF @@ -41,7 +55,7 @@ jobs: json-output: skillgate-review.json - uses: github/codeql-action/upload-sarif@v4 - if: always() + if: github.event_name != 'pull_request' && always() with: sarif_file: skillgate.sarif @@ -52,6 +66,7 @@ jobs: path: | skillgate-summary.md skillgate-review.json + skillgate.sarif ``` ## Blocking Policy Check With Policy-Aware SARIF @@ -92,7 +107,7 @@ jobs: json-output: skillgate-review.json - uses: github/codeql-action/upload-sarif@v4 - if: always() + if: github.event_name != 'pull_request' && always() with: sarif_file: skillgate.sarif @@ -103,6 +118,7 @@ jobs: path: | skillgate-summary.md skillgate-review.json + skillgate.sarif ``` ## Baseline Drift Blocking @@ -142,7 +158,7 @@ jobs: fail-on-drift: "true" - uses: github/codeql-action/upload-sarif@v4 - if: always() + if: github.event_name != 'pull_request' && always() with: sarif_file: skillgate.sarif @@ -153,6 +169,7 @@ jobs: path: | skillgate-summary.md skillgate-review.json + skillgate.sarif ``` ## Repository Plus Committed MCPB Bundle @@ -195,12 +212,12 @@ jobs: json-output: skillgate-review.json - uses: github/codeql-action/upload-sarif@v4 - if: always() + if: github.event_name != 'pull_request' && always() with: sarif_file: skillgate.sarif - uses: github/codeql-action/upload-sarif@v4 - if: always() + if: github.event_name != 'pull_request' && always() with: sarif_file: skillgate-mcpb.sarif @@ -211,4 +228,6 @@ jobs: path: | skillgate-summary.md skillgate-review.json + skillgate.sarif + skillgate-mcpb.sarif ``` diff --git a/docs/starter-repository.md b/docs/starter-repository.md new file mode 100644 index 0000000..23d7afe --- /dev/null +++ b/docs/starter-repository.md @@ -0,0 +1,21 @@ +# Pre-install starter repository + +Copy [`examples/preinstall-starter/`](../examples/preinstall-starter/) when a +new Agent Skill repository needs a safe first review. It demonstrates the full +adoption path without requiring a real third-party package or executing any +repository content. + +The local review command is the default path. It reads the checked-out files and +writes reports locally; it does not contact GitHub or upload findings. + +The GitHub workflow is a separate, optional integration with two deliberate +modes: + +- Pull requests retain Markdown, JSON, and SARIF as artifacts. Findings remain + reviewable but do not create a blocking Code Scanning status. +- Pushes to `main` and manual runs publish SARIF to Code Scanning for durable + visibility. + +After reviewers approve the observed capability surface, add a reviewed policy +to the Action or adopt a baseline with `fail-on-drift: "true"`. Keep that +enforcement decision separate from the advisory pre-install packet. diff --git a/examples/preinstall-starter/.github/workflows/skillgate-review.yml b/examples/preinstall-starter/.github/workflows/skillgate-review.yml new file mode 100644 index 0000000..74acf9b --- /dev/null +++ b/examples/preinstall-starter/.github/workflows/skillgate-review.yml @@ -0,0 +1,39 @@ +name: SkillGate pre-install review + +on: + workflow_dispatch: + pull_request: + push: + branches: [main] + +jobs: + review: + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + steps: + - uses: actions/checkout@v6 + - uses: charliechenye/SkillGate@v0 + with: + path: . + sarif-output: skillgate.sarif + summary-output: skillgate-review.md + json-output: skillgate-review.json + step-summary: "true" + + - name: Upload review artifacts + if: always() + uses: actions/upload-artifact@v7 + with: + name: skillgate-review + path: | + skillgate-review.md + skillgate-review.json + skillgate.sarif + + - name: Publish SARIF on protected branches and manual runs + if: github.event_name != 'pull_request' && always() + uses: github/codeql-action/upload-sarif@v4 + with: + sarif_file: skillgate.sarif diff --git a/examples/preinstall-starter/.python-version b/examples/preinstall-starter/.python-version new file mode 100644 index 0000000..e4fba21 --- /dev/null +++ b/examples/preinstall-starter/.python-version @@ -0,0 +1 @@ +3.12 diff --git a/examples/preinstall-starter/README.md b/examples/preinstall-starter/README.md new file mode 100644 index 0000000..bd6e9a1 --- /dev/null +++ b/examples/preinstall-starter/README.md @@ -0,0 +1,43 @@ +# SkillGate pre-install review starter + +This small repository is a copyable starting point for reviewing an Agent Skill +before it is installed or merged. It contains one intentionally boring Skill and +no helper scripts, package hooks, servers, or network configuration. + +## Local review + +From a SkillGate source checkout, run: + +```bash +uv sync --locked --group dev +uv run skillgate review preinstall examples/preinstall-starter/skills/safe-greeting \ + --json-output test-outputs/safe-greeting-review.json +``` + +Or, from this starter repository after installing SkillGate, run: + +```bash +skillgate review preinstall skills/safe-greeting \ + --json-output safe-greeting-review.json +``` + +The review is advisory by default. Add `--fail-on high` when a local check should +return a failing status for high or critical review signals. + +This local command is the default starter path. It writes only to the terminal +or the local `--json-output` path; it does not contact GitHub or upload reports. + +## Optional GitHub Actions integration + +The workflow in `.github/workflows/skillgate-review.yml` is optional and is not +used by the local commands above. Enable it only if you want GitHub-hosted CI. +When enabled, it keeps Markdown, JSON, and SARIF review files as pull-request +artifacts. It does not upload pull-request SARIF to Code Scanning. Pushes to +`main` and manual runs publish SARIF to GitHub Code Scanning. + +When the repository has reviewed behavior to enforce, add a `policy` or +`baseline` to the Action and use `fail-on-drift: "true"` as documented in the +[Action examples](../../docs/examples/github-action-minimal.md). + +The local review does not contact GitHub. SkillGate does not execute the Skill, +install packages, start servers, or call an agent during this review. diff --git a/examples/preinstall-starter/skills/safe-greeting/SKILL.md b/examples/preinstall-starter/skills/safe-greeting/SKILL.md new file mode 100644 index 0000000..8587edd --- /dev/null +++ b/examples/preinstall-starter/skills/safe-greeting/SKILL.md @@ -0,0 +1,12 @@ +--- +name: safe-greeting +description: Returns a short greeting without using tools or external resources. +license: MIT +compatibility: Any text-capable agent +allowed-tools: [] +--- + +# Safe greeting + +Return a short, friendly greeting when the user asks for one. Use no external +capabilities or tools. diff --git a/future_steps.md b/future_steps.md index bb0a7ce..2fd745a 100644 --- a/future_steps.md +++ b/future_steps.md @@ -2,372 +2,142 @@ ## Product Direction -SkillGate should remain a local-first, deterministic trust gate for AI-agent skills, MCP configurations, MCP bundles, and agent-tooling supply chains. +SkillGate is a local-first, deterministic trust gate for AI-agent skills, MCP +configurations, MCP bundles, and agent-tooling supply chains. Its clearest job +is to answer this question before install, merge, or approval: -The project should answer one practical question better than anyone else: +> What new capability surface does this artifact introduce? -> Before I install, merge, or approve this agent skill or MCP server, what new capability surface does it introduce? - -SkillGate should stay focused on pre-install and pre-merge review. It should not become a hosted dashboard, generic agent framework, runtime gateway, malware scanner, or broad observability platform until the local trust-gate workflow is widely useful and clearly differentiated. - -The roadmap should prioritize complete user workflows, distribution, and public proof over adding large numbers of loosely connected rules. - ---- +The roadmap prioritizes complete review workflows, low-friction distribution, +and public evidence over a large number of loosely connected rules. SkillGate +should not become a hosted dashboard, runtime gateway, malware scanner, or +generic agent framework without a deliberate product decision. ## Current Baseline -The current stable release is `v0.1.2`. - -The shipped baseline includes: - -- local static scans for agent-relevant files; -- sparse GitHub pre-install scans; -- deterministic rules for shell execution, destructive commands, network egress, remote download execution, secret access, filesystem writes, prompt override language, suspicious Unicode, and MCP-related risks; -- policy-as-code checks; -- expiring finding waivers; -- baseline creation and capability-drift detection; -- provenance verification for policy and baseline files; -- SARIF output and GitHub code-scanning integration; -- MCPB SARIF output and composite Action support for scanning committed MCPB artifacts; -- reviewer-friendly Markdown and JSON summaries; -- GitHub Step Summary support; -- structured MCP registry drift output; -- a composite GitHub Action that can enforce a supplied `baseline` plus `fail-on-drift`; -- a GitHub-first Node wrapper backed by checksummed release binaries; -- bounded release-manifest and binary downloads; -- a reusable, fail-closed ZIP archive inspection foundation organized into focused internal modules; -- the MCPB pre-install scan MVP with text and JSON output, manifest output, retained nested-archive inventory, embedded-artifact review findings, and no bundle execution; -- a packaged `skillgate demo mcpb` first-run demo that builds a deterministic synthetic bundle and can scan it immediately. -- the first `skillgate skills validate` workflow for deterministic Agent Skills structure and metadata checks. -- a packaged `skillgate demo skill` first-run demo that connects declared skill metadata to observed static capabilities. -- guided review sessions for first local review, pre-install review, and policy/baseline approval. - -These capabilities are release history, not future work. Keep detailed records in `CHANGELOG.md`. - ---- +The current stable release is `v0.1.2`. The shipped baseline includes: -## Near-Term Priority +- local and sparse GitHub static scans; +- deterministic findings for agent instructions, scripts, MCP metadata, and + capability risks; +- policy-as-code, waivers, baselines, capability drift, and provenance checks; +- text, JSON, SARIF, Markdown review summaries, and GitHub Step Summary output; +- MCP registry comparison and MCPB pre-install inspection without execution; +- a composite GitHub Action with policy, baseline, review artifact, and MCPB + support; +- a checksummed GitHub-first Node wrapper; +- `skillgate skills validate` and deterministic Agent Skill/MCPB demos; and +- guided review sessions for local review, pre-install review, and approval. -The next phase should reduce first-use friction, publish evidence, and expand MCPB only where real usage justifies it. +These are release history, not future work. Keep detailed records in +`CHANGELOG.md`. -The execution order is: +## 0.1.3 Adoption Priorities -1. Publish the Python package through a low-friction distribution path. -2. Publish credible public scan reports. -3. Simplify onboarding and demonstrate the workflow visually. -4. Expand the MCPB workflow only where real usage justifies it. -5. Expand Agent Skills validation and documentation where real usage justifies it. -6. Build the declared-intent-versus-observed-capability model. +The next minor release is a focused pre-install adoption release. The target is +one copy-pasteable review flow that produces a decision-ready packet without +executing code or making network requests for local inputs. -Do not add more generic rule families unless they are necessary to complete one of these workflows. +### 1. Standardize contributor setup ---- +Use the pinned `.python-version`, committed `uv.lock`, `uv sync --locked`, and +`uv run` as the canonical development workflow. CI and contributor docs should +use the same commands. -# Milestone 0.2.0: Safe MCPB Pre-Install Scanner +### 2. Add one unified pre-install review -## Product Outcome - -A user can inspect an MCP bundle before installing or executing it and receive a deterministic answer about: - -- what starts; -- which file is the entry point; -- which commands or runtimes are involved; -- which environment variables and secrets are referenced; -- which URLs and endpoints are declared; -- which files are bundled; -- whether binaries or nested archives require review; -- which existing SkillGate findings apply. - -The first public workflow should be: +Provide: ```bash -skillgate mcpb scan bundle.mcpb +skillgate review preinstall SOURCE ``` -# Batch 2B: Distribution And Adoption +The command should accept a local file or directory, a GitHub repository or +subtree URL, and a local `.mcpb` bundle. It should produce Markdown by default +and optional stable JSON containing source identity, capabilities, findings, +Agent Skills validation results, reviewer next actions, limitations, and the +no-execution guarantee. Results remain advisory unless the caller supplies an +explicit `--fail-on` threshold. -The scanner will not gain broad adoption through feature depth alone. Reduce first-use friction and publish evidence. +Existing enforcement remains in `check`, `diff`, and `review summary`; this +release does not add MCPB policy enforcement or declared-intent diffing. -## Publish The Python Package +### 3. Publish reproducible evidence -Target installation: +Add a Markdown benchmark report generated from the committed fixtures. Include +scanner version, fixture totals, rule coverage, expected-versus-actual results, +attribution, reproduction commands, and limitations. Do not present fixture +results as real-world detection accuracy. -```bash -pipx install openevalgate-skillgate -skillgate scan . -``` +Keep public scan reports in `docs/public-scan-reports/` and label findings as +review items unless there is evidence for a stronger claim. -Also validate: +### 4. Make first adoption copyable -```bash -uvx openevalgate-skillgate scan . -``` - -Before publication: - -- verify package ownership and the final distribution name; -- verify README metadata and project URLs; -- build source and wheel distributions; -- install from a clean environment; -- test all CLI entry points and version output; -- test upgrade behavior; -- verify dependency bounds; -- publish to a test index when appropriate; -- perform post-publication smoke tests; -- document rollback and yanking procedures. +Add a starter repository with a minimal safe Agent Skill and a first-run local +pre-install review command. An optional GitHub Action may retain Markdown, JSON, +and SARIF as artifacts. If a repository owner enables that integration, +main-branch and manual runs may publish SARIF to Code Scanning; pull requests +should remain reviewable without making intentional fixture findings a blocking +status. -Keep GitHub tags and full commit SHA installation documented for high-trust users. - -## Publish Three Public Scan Reports - -Create: +The documented escalation path is: ```text -docs/public-scan-reports/ -``` - -Publish: - -1. a clean agent skill repository; -2. a repository with meaningful shell, network, secret, or helper-script review items; -3. an MCPB bundle report. - -Each report should include: - -- immutable source revision or archive hash; -- exact command used; -- scanner version; -- capability inventory; -- findings summary; -- classification of each finding as expected behavior, review item, or demonstrated vulnerability; -- limitations; -- suggested policy; -- what SkillGate cannot conclude. - -Do not shame maintainers or label findings as vulnerabilities without evidence. - -## Simplify README Onboarding - -The top-level README should initially present three workflows: - -### Before installing - -```bash -skillgate github scan URL -skillgate mcpb scan bundle.mcpb -``` - -### Before merging - -```bash -skillgate scan . -``` - -### When ready to enforce - -```bash -skillgate check . --policy skillgate.yaml -``` - -Move baselines, provenance, inventory, MCP registry comparison, SARIF details, and advanced Action settings below the primary onboarding path. - -## Add A Visual Demo - -Publish a short terminal recording or animated demonstration that shows: - -```text -bundle.mcpb -→ archive inspected -→ entry point detected -→ startup command detected -→ secret and endpoint references identified -→ review result produced -``` - -The demo should use a committed deterministic fixture. - -## Adoption Signals - -Track: - -- repository stars; -- unique cloners; -- package downloads; -- GitHub Action usage; -- external issues and pull requests; -- public references; -- successful scans of third-party repositories; -- number of public scan reports; -- time from installation to first useful result. - -Do not add telemetry to the CLI without an explicit privacy design and opt-in decision. - ---- - -# Batch 2C: MCPB Workflow Expansion - -Only begin this batch after the MCPB text and JSON output contracts have been exercised through fixtures and public reports. - -Potential additions: - -- policy controls for bundle-specific findings; -- richer package metadata interpretation; -- declared file inventory comparisons; -- stronger unexpected-file detection; -- example CI workflows; -- additional public bundle reports. - -Do not add all of these automatically. Prioritize based on observed user requests. - ---- - -# Milestone 0.3.0: Agent Skills Standards Alignment - -## Product Outcome - -A user can validate whether a skill is structurally complete and compare its declared intent with the capabilities detected in its files and scripts. - -## Add Skills Validation - -Ship: - -```bash -skillgate skills validate PATH +review preinstall → review summary → check with policy → diff with baseline ``` -Validate: - -- required `name` and `description`; -- name format; -- parent-directory naming consistency; -- optional `license`, `compatibility`, and metadata; -- experimental `allowed-tools`; -- supported optional directories: `scripts/`, `references/`, and `assets/`. - -Report malformed frontmatter, missing required fields, invalid names, directory-name mismatches, missing referenced files, missing license metadata, ambiguous compatibility declarations, executables outside expected directories, missing referenced scripts or assets, and broad or ambiguous `allowed-tools`. - -Treat `allowed-tools` as declared metadata, not proof of runtime enforcement. - -## Required Evidence - -Ship with: +### 5. Keep distribution decisions explicit -- valid minimal skill; -- valid complex skill; -- malformed frontmatter; -- missing referenced script; -- hidden executable; -- broad `allowed-tools`; -- one public standards-aligned scan report. +PyPI publication is deferred for `0.1.3`. When publication is revisited, use +the existing distribution name `openevalgate-skillgate`; the `skillgate` PyPI +name is already occupied by another project. Do not rename the Python package +or publish a second distribution as a workaround. ---- +The root npm package remains private and npm publication is also deferred. The +GitHub-first Node wrapper remains the supported Node entry point until a package +name, ownership, and publication strategy are intentionally approved. -# Milestone 0.4.0: Declared Intent Versus Observed Capability - -This should become SkillGate’s strongest long-term differentiator. - -## Product Outcome - -SkillGate should explain where an agent artifact’s stated purpose or declared permissions do not match the capabilities observed in its implementation. - -Add: +The supported source-checkout example remains: ```bash -skillgate skills diff PATH +npx --yes github:charliechenye/SkillGate#v0 -- scan . ``` -Compare declared tools, compatibility, environment variables, network access, and filesystem behavior with detected shell commands, network hosts, secret names, filesystem writes, executable references, MCP server references, and package scripts. +## Later Work -Report: +### Declared intent versus observed capability -- observed but undeclared capabilities; -- declared but unused capabilities; -- newly introduced capabilities; -- removed capabilities; -- capability severity changes; -- missing scripts, references, or assets; -- contradictions between description and observed behavior. +After the unified review flow has real usage, add an explainable comparison of +declared tools and metadata against capabilities observed in files and scripts. +This should report undeclared capabilities, unused declarations, and +contradictions without inferring intent speculatively. -Core concept: +### Targeted MCPB and Agent Skills expansion -> Declared intent versus observed capability. +Expand MCPB policy controls and Agent Skills validation only in response to +real adoption evidence. Do not add new rule families or rule IDs merely to make +the roadmap look larger. -Policy hooks may later allow: +## Maintenance Requirements -```yaml -policy: - skills: - require_declared_capabilities: true - block_undeclared_high_risk_capabilities: true -``` +Keep the documented Action behavior stable for advisory scans, blocking policy +checks, baseline drift blocking, GitHub Step Summary, Markdown summaries, JSON +artifacts, and SARIF review artifacts. Preserve copy-pasteable examples for: -This milestone should focus on explainable mismatches, not speculative intent inference. - ---- - -# Maintenance Requirements - -## Maintain GitHub Action Reliability - -Keep the documented Action behavior stable for nonblocking scans, blocking policy checks, policy-aware SARIF, baseline drift blocking, GitHub Step Summary, Markdown summaries, and machine-readable review artifacts. - -Maintain copy-paste examples for: - -- nonblocking scan with SARIF; +- nonblocking scan with SARIF retained as a pull-request artifact; - blocking policy check with policy-aware SARIF; -- baseline drift blocking; +- baseline drift blocking; and - review-summary artifact upload. -## Maintain Release Verification - -For every release: - -- verify the concrete release tag and stable `v0` tag; -- verify GitHub Action usage from a clean external repository; -- verify GitHub-tag and package installation paths; -- verify the Node wrapper from a clean environment; -- verify binary hashes and manifest sizes; -- verify unsupported-platform failures; -- verify offline cached-binary behavior. - -Continue recommending full commit SHA pinning for high-trust GitHub Action users, explicit release tags for Python installation, and explicit `SKILLGATE_VERSION` for Node wrapper downloads. - -## Maintain GitHub-First Node Distribution - -Keep the Python implementation canonical. Do not maintain a second scanner in TypeScript. - -Current intended Node usage: +The composite Action can enforce a supplied `baseline` plus `fail-on-drift` when +a repository explicitly opts into that gate. -```bash -npx --yes github:charliechenye/SkillGate#v0 -- scan . -``` - -Maintain platform-specific binary selection, release-manifest checksum validation, bounded downloads, cache support, offline cached-binary mode, explicit version pinning, and clear unsupported-platform messages. - -Do not promote bare `npx skillgate scan .` until an npm package is intentionally published. - ---- +Do not add telemetry without an explicit privacy design and opt-in decision. -# Prioritization Rules - -1. Prefer a complete user workflow over another isolated capability. -2. Prefer deterministic evidence over heuristic breadth. -3. Prefer reuse of the existing scanner over parallel implementations. -4. Prefer adoption friction reduction over advanced configuration. -5. Prefer public examples over unsupported security claims. -6. Add new rule IDs only when existing rules cannot express the review decision. -7. Keep stable output contracts small and versioned. -8. Treat cleanup, bounded resource use, provenance, and no-execution guarantees as product requirements. -9. Keep `future_steps.md` future-only. -10. Record completed work under the next release section in `CHANGELOG.md`. - ---- - -# Immediate Execution Order - -```text -release: prepare low-friction Python distribution -docs: publish public scan evidence and simplify onboarding -``` +## Release Checklist References -This order keeps the project focused on user-visible adoption now that the MCPB pre-install scan MVP is part of the baseline. +The release checklist covers the `v0.1.2` launch history and remains the source +for tag and binary verification details. For the next release, use the same +checks with `uv sync --locked`, the generated benchmark report, the starter +repository smoke test, and the final no-execution review. diff --git a/pyproject.toml b/pyproject.toml index a3ce238..49f8332 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -53,12 +53,6 @@ dependencies = [ "typer>=0.12", ] -[project.optional-dependencies] -dev = [ - "pytest>=8.0", - "ruff>=0.5", -] - [project.scripts] skillgate = "skillgate.cli:app" @@ -86,7 +80,7 @@ skillgate = [ [tool.pytest.ini_options] testpaths = ["tests"] -pythonpath = ["src"] +pythonpath = [".", "src"] addopts = "-p no:cacheprovider" [tool.ruff] @@ -110,5 +104,7 @@ quote-style = "double" [dependency-groups] dev = [ + "pytest>=8.0", + "ruff>=0.5", "twine>=6.2.0", ] diff --git a/src/skillgate/cli.py b/src/skillgate/cli.py index 7e6e2fe..03669e0 100644 --- a/src/skillgate/cli.py +++ b/src/skillgate/cli.py @@ -2,6 +2,7 @@ from pathlib import Path from typing import Annotated +from urllib.parse import urlsplit import typer from rich.console import Console @@ -16,6 +17,7 @@ ) from skillgate.fixtures import ( FixtureSummaryError, + fixture_summary_markdown, fixture_summary_payload, fixture_summary_text, summarize_fixtures, @@ -32,10 +34,21 @@ from skillgate.mcpb.errors import McpbError, mcpb_archive_error_data from skillgate.mcpb.reporting import mcpb_manifest_json, mcpb_scan_json, mcpb_scan_text from skillgate.mcpb.scan import scan_mcpb -from skillgate.models import SEVERITY_ORDER, DiffReport, severity_at_or_above, stable_json +from skillgate.models import ( + SEVERITY_ORDER, + DiffReport, + model_to_data, + severity_at_or_above, + stable_json, +) from skillgate.policy import evaluate_policy, load_policy from skillgate.policy_schema import POLICY_JSON_SCHEMA from skillgate.policy_templates import POLICY_PROFILES, policy_template_yaml +from skillgate.preinstall import ( + build_preinstall_packet, + preinstall_packet_json, + render_preinstall_markdown, +) from skillgate.provenance import ( ProvenanceError, create_provenance_manifest, @@ -54,9 +67,10 @@ ) from skillgate.review import render_review_markdown, review_summary_payload from skillgate.rule_docs import RULE_DOCS, get_rule_doc, rule_doc_to_data, rule_docs_to_data -from skillgate.scan import filter_report_by_severity, scan_repository +from skillgate.scan import filter_report_by_severity, scan_paths, scan_repository from skillgate.skills import ( SkillsValidationError, + discover_skill_files, skills_failed, skills_json, skills_text, @@ -176,6 +190,140 @@ def render_scan_command_output( return content, failed +def _preinstall_skills(path: Path) -> dict[str, object] | None: + try: + if path.is_file() and path.name != "SKILL.md": + return None + if not discover_skill_files(path): + return None + return validate_skills(path) + except SkillsValidationError as exc: + if "no SKILL.md files found" in str(exc): + return None + raise + + +def _preinstall_failed(packet: dict[str, object], fail_on: str | None) -> bool: + if fail_on is None: + return False + findings = packet["findings"] + return any( + severity_at_or_above(item["severity"], fail_on) + for items in findings["groups"].values() + for item in items + ) + + +def _is_github_url(value: str) -> bool: + parsed = urlsplit(value) + return parsed.scheme in {"http", "https"} and parsed.netloc.lower() == "github.com" + + +@review_app.command("preinstall") +def review_preinstall( + source: Annotated[ + str, + typer.Argument(help="Local path, GitHub repository URL, or local .mcpb bundle."), + ], + output_format: Annotated[ + str, + typer.Option("--format", help="Output format: markdown or json."), + ] = "markdown", + fail_on: Annotated[ + str | None, + typer.Option( + "--fail-on", + help="Exit 1 when scan or validation findings reach this severity.", + ), + ] = None, + output: Annotated[ + Path | None, + typer.Option("--output", "-o", help="Write the primary review packet."), + ] = None, + json_output: Annotated[ + Path | None, + typer.Option("--json-output", help="Also write the machine-readable packet JSON."), + ] = None, +) -> None: + """Create an advisory review packet before installing an agent artifact.""" + output_format = validate_format(output_format, {"markdown", "json"}) + fail_on = validate_skills_fail_on(fail_on) + if output and json_output and output.resolve() == json_output.resolve(): + console.file.write("Error: --output and --json-output must be different paths\n") + raise typer.Exit(2) + + sparse = None + try: + if _is_github_url(source): + sparse = fetch_github_sparse(source) + scan_path = sparse.root + scan_report = scan_repository(scan_path) + skills_payload = _preinstall_skills(scan_path) + packet = build_preinstall_packet( + { + "kind": "github", + "reference": source, + "revision": sparse.manifest.get("commit_sha") + or sparse.manifest.get("resolved_ref"), + "metadata": sparse.manifest, + }, + scan_report, + skills_payload, + ) + else: + path = Path(source).expanduser().resolve() + if not path.exists(): + raise ValueError(f"source does not exist: {path}") + if path.suffix.lower() == ".mcpb": + result = scan_mcpb(path) + packet = build_preinstall_packet( + { + "kind": "mcpb", + "reference": str(path), + "path": str(path), + "digest": result.bundle_manifest.archive.sha256, + "metadata": model_to_data(result.bundle_manifest), + }, + result.scan_report, + ) + else: + scan_report = ( + scan_paths(path.parent, [path]) if path.is_file() else scan_repository(path) + ) + packet = build_preinstall_packet( + { + "kind": "local", + "reference": str(path), + "path": str(path), + "metadata": {"input_type": "directory" if path.is_dir() else "file"}, + }, + scan_report, + _preinstall_skills(path), + ) + except ( + ArchiveError, + McpbError, + OSError, + SourceError, + SkillsValidationError, + ValueError, + ) as exc: + console.file.write(f"Error: {exc}\n") + raise typer.Exit(2) from exc + finally: + if sparse is not None: + sparse.cleanup() + + json_content = preinstall_packet_json(packet) + if json_output: + write_or_print(json_content, json_output, console) + content = json_content if output_format == "json" else render_preinstall_markdown(packet) + if _preinstall_failed(packet, fail_on) and output_format == "markdown": + content += f"\nReview threshold failed: findings at or above `{fail_on}`.\n" + write_or_print(content, output, console) + raise typer.Exit(1 if _preinstall_failed(packet, fail_on) else 0) + + @demo_app.command("mcpb") def demo_mcpb( output: Annotated[ @@ -676,7 +824,7 @@ def fixtures_summary( output_format: Annotated[str, typer.Option("--format", help="Output format.")] = "json", ) -> None: """Summarize benchmark fixture expectations and actual findings.""" - output_format = validate_format(output_format, {"text", "json"}) + output_format = validate_format(output_format, {"text", "json", "markdown"}) try: summaries = summarize_fixtures(path) except FixtureSummaryError as exc: @@ -684,6 +832,8 @@ def fixtures_summary( raise typer.Exit(2) from exc if output_format == "json": console.file.write(stable_json(fixture_summary_payload(path, summaries))) + elif output_format == "markdown": + console.file.write(fixture_summary_markdown(path, summaries)) else: console.file.write(fixture_summary_text(path, summaries)) raise typer.Exit(1 if any(summary.status == "fail" for summary in summaries) else 0) diff --git a/src/skillgate/fixtures.py b/src/skillgate/fixtures.py index ec2664a..34f3f76 100644 --- a/src/skillgate/fixtures.py +++ b/src/skillgate/fixtures.py @@ -5,6 +5,7 @@ import yaml +from skillgate import __version__ from skillgate.baseline import create_baseline, diff_against_baseline from skillgate.scan import scan_repository @@ -152,3 +153,80 @@ def fixture_summary_text(root: Path, summaries: list[FixtureSummary]) -> str: ] ) return "\n".join(lines) + "\n" + + +def fixture_summary_markdown(root: Path, summaries: list[FixtureSummary]) -> str: + expected_rules = sorted( + {rule_id for summary in summaries for rule_id in summary.expected_rule_ids} + ) + actual_rules = sorted( + { + rule_id + for summary in summaries + for rule_id in [*summary.actual_rule_ids, *summary.diff_rule_ids] + } + ) + lines = [ + "# SkillGate Benchmark Report", + "", + f"- Scanner version: `{__version__}`", + f"- Fixture root: `{root.as_posix()}`", + f"- Fixtures: {len(summaries)}", + f"- Passed: {sum(1 for summary in summaries if summary.status == 'pass')}", + f"- Failed: {sum(1 for summary in summaries if summary.status == 'fail')}", + "", + "## Rule Coverage", + "", + "| Rule | Expected fixtures | Actual fixtures | Coverage status |", + "| --- | ---: | ---: | --- |", + ] + for rule_id in sorted(set(expected_rules) | set(actual_rules)): + expected_count = sum(rule_id in summary.expected_rule_ids for summary in summaries) + actual_count = sum( + rule_id in summary.actual_rule_ids or rule_id in summary.diff_rule_ids + for summary in summaries + ) + status = "covered" if expected_count == actual_count else "mismatch" + lines.append(f"| {rule_id} | {expected_count} | {actual_count} | {status} |") + lines.extend( + [ + "", + "## Fixture Results", + "", + "| Fixture | Status | Expected rules | Actual rules | Attribution |", + "| --- | --- | --- | --- | --- |", + ] + ) + for summary in summaries: + actual = sorted(set(summary.actual_rule_ids) | set(summary.diff_rule_ids)) + attribution = "yes" if summary.attribution else "no" + lines.append( + f"| `{summary.name}` | **{summary.status}** | " + f"`{', '.join(summary.expected_rule_ids) or 'none'}` | " + f"`{', '.join(actual) or 'none'}` | {attribution} |" + ) + lines.extend( + [ + "", + "## Reproduce", + "", + "```bash", + "skillgate fixtures summary fixtures/benchmark --format markdown", + "```", + "", + "## Attribution", + "", + "Public-pattern fixtures retain source URLs, retrieval dates, and reduction " + "notes in their expected-findings metadata.", + "", + "## Limitations", + "", + "This report verifies deterministic fixture expectations and rule coverage. It " + "is not a real-world detection accuracy benchmark, malware verdict, or " + "completeness claim.", + "Fixtures are intentionally reduced examples and may not represent the " + "behavior, context, or security posture of their source projects.", + "", + ] + ) + return "\n".join(lines) diff --git a/src/skillgate/preinstall.py b/src/skillgate/preinstall.py new file mode 100644 index 0000000..fc7db66 --- /dev/null +++ b/src/skillgate/preinstall.py @@ -0,0 +1,304 @@ +"""Stable, redacted review packets for pre-install decisions.""" + +from __future__ import annotations + +import re +from pathlib import Path +from typing import Any +from urllib.parse import urlsplit, urlunsplit + +from skillgate import __version__ +from skillgate.inventory import normalized_resource, trust_boundary_for +from skillgate.models import ( + SEVERITY_ORDER, + Capability, + Finding, + ScanReport, + model_to_data, + stable_json, +) + +PREINSTALL_PACKET_SCHEMA_VERSION = "1" +_SECRET_ASSIGNMENT_RE = re.compile( + r"(?i)(\b(?:token|secret|password|credential|api[_-]?key|access[_-]?key)\b\s*[:=]\s*)([^\s,;]+)" +) + + +def _redact_url(value: str) -> str: + try: + parsed = urlsplit(value) + except ValueError: + return value + if not parsed.username and not parsed.password: + return value + host = parsed.hostname or "" + if parsed.port: + host = f"{host}:{parsed.port}" + return urlunsplit( + (parsed.scheme, "[REDACTED]@" + host, parsed.path, parsed.query, parsed.fragment) + ) + + +def _redact_text(value: str) -> str: + value = _redact_url(value) + return _SECRET_ASSIGNMENT_RE.sub(r"\1[REDACTED]", value) + + +def _redact_path(value: str | None, root: str | None = None) -> str | None: + if value is None: + return None + normalized = value.replace("\\", "/") + if root: + root_path = Path(root).expanduser() + try: + candidate = Path(normalized) + if candidate.is_absolute(): + return candidate.relative_to(root_path).as_posix() + except ValueError: + pass + if Path(normalized).is_absolute(): + return f"/{Path(normalized).name}" + return normalized + + +def _source_record(source: dict[str, Any], root: str | None) -> dict[str, Any]: + reference = str(source.get("reference", "")) + record = { + "kind": source.get("kind", "unknown"), + "reference": ( + _redact_path(reference, root) + if source.get("kind") == "local" + else _redact_url(reference) + ), + } + if source.get("path") is not None: + record["path"] = _redact_path(str(source["path"]), root) + for key in ("revision", "subpath", "digest"): + if source.get(key) is not None: + record[key] = _redact_text(str(source[key])) + if source.get("metadata"): + record["metadata"] = _redact_mapping(source["metadata"], root) + return record + + +def _redact_mapping(value: Any, root: str | None = None, key: str = "") -> Any: + if isinstance(value, dict): + return { + str(name): _redact_mapping(item, root, str(name)) + for name, item in sorted(value.items()) + } + if isinstance(value, list): + return [_redact_mapping(item, root, key) for item in value] + if isinstance(value, str): + lowered_key = key.lower() + if any(token in lowered_key for token in ("token", "secret", "password", "credential")): + return "[REDACTED]" + if any(token in key.lower() for token in ("path", "file", "root")): + return _redact_path(value, root) + return _redact_text(value) + return value + + +def _finding_record(finding: Finding | dict[str, Any], root: str | None = None) -> dict[str, Any]: + data = model_to_data(finding) + return { + "id": data.get("id"), + "rule_id": data.get("rule_id") or data.get("code"), + "title": _redact_text(str(data.get("title", ""))), + "severity": data.get("severity", "informational"), + "capability": data.get("capability"), + "file_path": _redact_path(data.get("file_path"), root), + "line_number": data.get("line_number"), + "evidence": _redact_text(str(data["evidence"])) if data.get("evidence") else None, + "remediation": _redact_text(str(data["remediation"])) if data.get("remediation") else None, + } + + +def _capability_record( + capability: Capability | dict[str, Any], root: str | None = None +) -> dict[str, Any]: + data = model_to_data(capability) + return { + "type": data.get("type"), + "resource": _redact_text(normalized_resource(data.get("resource")) or ""), + "source_file": _redact_path(data.get("source_file"), root), + "source_line": data.get("source_line"), + "trust_boundary": trust_boundary_for(data.get("type", "")) or "other", + } + + +def _severity_groups(findings: list[dict[str, Any]]) -> dict[str, list[dict[str, Any]]]: + grouped = {severity: [] for severity in SEVERITY_ORDER} + for finding in findings: + grouped.setdefault(finding["severity"], []).append(finding) + for values in grouped.values(): + values.sort( + key=lambda item: ( + SEVERITY_ORDER.get(item["severity"], -1), + item["rule_id"] or "", + item["file_path"] or "", + ) + ) + return grouped + + +def build_preinstall_packet( + source: dict[str, Any], + scan_report: ScanReport | dict[str, Any] | None = None, + skills_payload: dict[str, Any] | None = None, + metadata: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Build a deterministic review packet from scan and validation results.""" + report_data = model_to_data(scan_report) if scan_report is not None else {} + root = report_data.get("scan_root") if report_data else source.get("path") + findings = [_finding_record(item, root) for item in report_data.get("findings", [])] + skill_findings = [] + if skills_payload: + skill_findings = [ + _finding_record(item, skills_payload.get("root")) + for item in skills_payload.get("findings", []) + ] + all_findings = sorted( + [*findings, *skill_findings], + key=lambda item: ( + SEVERITY_ORDER.get(item["severity"], -1), + item["rule_id"] or "", + item["file_path"] or "", + item["line_number"] or 0, + ), + ) + grouped = _severity_groups(all_findings) + counts = {severity: len(grouped[severity]) for severity in SEVERITY_ORDER} + source_kind = source.get("kind", "unknown") + next_actions = [ + "Review each high or critical finding before installation.", + "Confirm that endpoints, secret references, and startup behavior are expected.", + "Use `skillgate check` or `skillgate diff` when this review becomes " + "an enforcement decision.", + ] + if not all_findings: + next_actions.insert( + 0, "No static findings were produced; continue with normal maintainer review." + ) + limitations = [ + "This is deterministic static analysis, not a malware verdict or runtime proof.", + "SkillGate does not execute code, install packages, start servers, or invoke an agent.", + "Findings identify review signals; they do not establish maintainer intent " + "or exploitability.", + ] + packet = { + "packet_type": "preinstall_review", + "schema_version": PREINSTALL_PACKET_SCHEMA_VERSION, + "tool_version": __version__, + "source": _source_record(source, root), + "metadata": _redact_mapping(metadata or {}, root), + "capabilities": [ + _capability_record(item, root) for item in report_data.get("capabilities", []) + ], + "findings": { + "total": len(all_findings), + "by_severity": counts, + "groups": grouped, + }, + "skills": { + "validated": skills_payload is not None, + "summary": _redact_mapping( + (skills_payload or {}).get("summary", {}), + skills_payload.get("root") if skills_payload else root, + ), + "findings": skill_findings, + }, + "reviewer": { + "decision": "review_required" if all_findings else "no_findings", + "next_actions": next_actions, + "limitations": limitations, + "no_execution": True, + "network_access": source_kind == "github", + }, + } + return _redact_mapping(packet, root) + + +def preinstall_packet_json(packet: dict[str, Any]) -> str: + return stable_json(packet) + + +def _table(headers: list[str], rows: list[list[Any]]) -> list[str]: + if not rows: + return ["None."] + lines = ["| " + " | ".join(headers) + " |", "| " + " | ".join("---" for _ in headers) + " |"] + for row in rows: + lines.append("| " + " | ".join(str(item or "").replace("|", "\\|") for item in row) + " |") + return lines + + +def render_preinstall_markdown(packet: dict[str, Any]) -> str: + source = packet["source"] + findings = packet["findings"] + lines = [ + "# SkillGate Pre-install Review", + "", + f"- Source: `{source['reference']}`", + f"- Kind: `{source['kind']}`", + f"- Decision: **{packet['reviewer']['decision']}**", + f"- Tool version: `{packet['tool_version']}`", + "", + "## Capability Inventory", + *( + _table( + ["Capability", "Resource", "Trust boundary", "Source"], + [ + [ + item["type"], + item["resource"], + item["trust_boundary"], + f"{item['source_file']}:{item['source_line'] or 1}", + ] + for item in packet["capabilities"] + ], + ) + ), + "", + "## Findings By Severity", + *( + _table( + ["Severity", "Rule", "Title", "Source"], + [ + [ + item["severity"], + item["rule_id"], + item["title"], + f"{item['file_path']}:{item['line_number'] or 1}", + ] + for item in sum(findings["groups"].values(), []) + ], + ) + ), + "", + "## Agent Skills Validation", + f"Validated: **{packet['skills']['validated']}**", + "", + *( + _table( + ["Severity", "Rule", "Title", "Source"], + [ + [ + item["severity"], + item["rule_id"], + item["title"], + f"{item['file_path']}:{item['line_number'] or 1}", + ] + for item in packet["skills"]["findings"] + ], + ) + ), + "", + "## Reviewer Next Actions", + *[f"- {item}" for item in packet["reviewer"]["next_actions"]], + "", + "## Limitations", + *[f"- {item}" for item in packet["reviewer"]["limitations"]], + "", + "No code was executed by the packet renderer.", + ] + return "\n".join(lines) + "\n" diff --git a/tests/snapshots/preinstall_packet.json b/tests/snapshots/preinstall_packet.json new file mode 100644 index 0000000..e00886b --- /dev/null +++ b/tests/snapshots/preinstall_packet.json @@ -0,0 +1,128 @@ +{ + "capabilities": [ + { + "resource": "example.com", + "source_file": "scripts/install.sh", + "source_line": 2, + "trust_boundary": "remote_endpoints", + "type": "network_egress" + }, + { + "resource": "example.com", + "source_file": "scripts/install.sh", + "source_line": 2, + "trust_boundary": "local_execution", + "type": "remote_download_execution" + }, + { + "resource": "", + "source_file": "scripts/install.sh", + "source_line": 1, + "trust_boundary": "local_execution", + "type": "shell_execution" + }, + { + "resource": "", + "source_file": "scripts/install.sh", + "source_line": 2, + "trust_boundary": "local_execution", + "type": "shell_execution" + } + ], + "findings": { + "by_severity": { + "critical": 0, + "high": 2, + "informational": 0, + "low": 0, + "medium": 2 + }, + "groups": { + "critical": [], + "high": [ + { + "capability": "shell_execution", + "evidence": "curl https://example.com/bootstrap.sh | bash", + "file_path": "scripts/install.sh", + "id": "SG001-3aa90c5e891b", + "line_number": 2, + "remediation": "Review whether shell execution is necessary and policy-approved.", + "rule_id": "SG001", + "severity": "high", + "title": "Shell execution detected" + }, + { + "capability": "remote_download_execution", + "evidence": "curl https://example.com/bootstrap.sh | bash", + "file_path": "scripts/install.sh", + "id": "SG004-d2f7e9d0d638", + "line_number": 2, + "remediation": "Pin and review downloaded artifacts before execution.", + "rule_id": "SG004", + "severity": "high", + "title": "Remote download followed by execution" + } + ], + "informational": [], + "low": [], + "medium": [ + { + "capability": "shell_execution", + "evidence": "#!/usr/bin/env bash", + "file_path": "scripts/install.sh", + "id": "SG001-ce951ab50926", + "line_number": 1, + "remediation": "Review whether shell execution is necessary and policy-approved.", + "rule_id": "SG001", + "severity": "medium", + "title": "Shell execution detected" + }, + { + "capability": "network_egress", + "evidence": "Host: example.com", + "file_path": "scripts/install.sh", + "id": "SG003-3a595d8bbd56", + "line_number": 2, + "remediation": "Allowlist the host or remove the network access.", + "rule_id": "SG003", + "severity": "medium", + "title": "Network egress detected" + } + ] + }, + "total": 4 + }, + "metadata": {}, + "packet_type": "preinstall_review", + "reviewer": { + "decision": "review_required", + "limitations": [ + "This is deterministic static analysis, not a malware verdict or runtime proof.", + "SkillGate does not execute code, install packages, start servers, or invoke an agent.", + "Findings identify review signals; they do not establish maintainer intent or exploitability." + ], + "network_access": false, + "next_actions": [ + "Review each high or critical finding before installation.", + "Confirm that endpoints, secret references, and startup behavior are expected.", + "Use `skillgate check` or `skillgate diff` when this review becomes an enforcement decision." + ], + "no_execution": true + }, + "schema_version": "1", + "skills": { + "findings": [], + "summary": {}, + "validated": false + }, + "source": { + "kind": "local", + "metadata": { + "owner_path": "/private", + "token": "[REDACTED]" + }, + "path": "/05-remote-download-execute", + "reference": "/05-remote-download-execute" + }, + "tool_version": "0.1.2" +} diff --git a/tests/snapshots/preinstall_packet.txt b/tests/snapshots/preinstall_packet.txt new file mode 100644 index 0000000..7fdf18f --- /dev/null +++ b/tests/snapshots/preinstall_packet.txt @@ -0,0 +1,39 @@ +# SkillGate Pre-install Review + +- Source: `/05-remote-download-execute` +- Kind: `local` +- Decision: **review_required** +- Tool version: `0.1.2` + +## Capability Inventory +| Capability | Resource | Trust boundary | Source | +| --- | --- | --- | --- | +| network_egress | example.com | remote_endpoints | scripts/install.sh:2 | +| remote_download_execution | example.com | local_execution | scripts/install.sh:2 | +| shell_execution | | local_execution | scripts/install.sh:1 | +| shell_execution | | local_execution | scripts/install.sh:2 | + +## Findings By Severity +| Severity | Rule | Title | Source | +| --- | --- | --- | --- | +| high | SG001 | Shell execution detected | scripts/install.sh:2 | +| high | SG004 | Remote download followed by execution | scripts/install.sh:2 | +| medium | SG001 | Shell execution detected | scripts/install.sh:1 | +| medium | SG003 | Network egress detected | scripts/install.sh:2 | + +## Agent Skills Validation +Validated: **False** + +None. + +## Reviewer Next Actions +- Review each high or critical finding before installation. +- Confirm that endpoints, secret references, and startup behavior are expected. +- Use `skillgate check` or `skillgate diff` when this review becomes an enforcement decision. + +## Limitations +- This is deterministic static analysis, not a malware verdict or runtime proof. +- SkillGate does not execute code, install packages, start servers, or invoke an agent. +- Findings identify review signals; they do not establish maintainer intent or exploitability. + +No code was executed by the packet renderer. diff --git a/tests/test_adoption_workflow.py b/tests/test_adoption_workflow.py new file mode 100644 index 0000000..05a6ca1 --- /dev/null +++ b/tests/test_adoption_workflow.py @@ -0,0 +1,119 @@ +from __future__ import annotations + +import json +from pathlib import Path + +import yaml +from conftest import FIXTURES, ROOT, runner + +from skillgate.cli import app +from skillgate.demo import build_demo_mcpb +from skillgate.fixtures import fixture_summary_markdown, summarize_fixtures + +STARTER = ROOT / "examples" / "preinstall-starter" + + +class FakeSparseResult: + def __init__(self, root: Path) -> None: + self.root = root + self.manifest = { + "resolved_ref": "main", + "commit_sha": "0123456789abcdef0123456789abcdef01234567", + "fetched_files": ["SKILL.md"], + } + self.cleaned = False + + def cleanup(self) -> None: + self.cleaned = True + + +def test_starter_repository_has_clean_unified_review() -> None: + result = runner.invoke(app, ["review", "preinstall", str(STARTER), "--format", "json"]) + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["source"]["kind"] == "local" + assert payload["skills"]["validated"] is True + assert payload["skills"]["summary"]["findings"] == 0 + assert payload["findings"]["by_severity"]["high"] == 0 + assert payload["reviewer"]["decision"] in {"no_findings", "review_required"} + + +def test_mocked_github_review_uses_immutable_manifest(monkeypatch, tmp_path: Path) -> None: + (tmp_path / "SKILL.md").write_text( + "---\nname: remote-safe\ndescription: A safe remote fixture.\n---\n", encoding="utf-8" + ) + fake = FakeSparseResult(tmp_path) + monkeypatch.setattr("skillgate.cli.fetch_github_sparse", lambda _url: fake) + result = runner.invoke( + app, + [ + "review", + "preinstall", + "https://github.com/example/repo/tree/main/skills", + "--format", + "json", + ], + ) + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["source"]["kind"] == "github" + assert payload["source"]["revision"] == fake.manifest["commit_sha"] + assert payload["source"]["metadata"]["fetched_files"] == ["SKILL.md"] + assert fake.cleaned is True + + +def test_mcpb_review_and_explicit_fail_on_are_advisory_then_enforceable(tmp_path: Path) -> None: + bundle = tmp_path / "reviewable.mcpb" + build_demo_mcpb(bundle) + bundle_result = runner.invoke( + app, + ["review", "preinstall", str(bundle), "--format", "json"], + ) + assert bundle_result.exit_code == 0 + assert json.loads(bundle_result.output)["source"]["kind"] == "mcpb" + + finding_result = runner.invoke( + app, + [ + "review", + "preinstall", + str(FIXTURES / "05-remote-download-execute"), + "--fail-on", + "high", + ], + ) + assert finding_result.exit_code == 1 + assert "review_required" in finding_result.output + assert "Review threshold failed" in finding_result.output + + +def test_benchmark_report_and_workflow_keep_pr_sarif_nonblocking() -> None: + benchmark_root = Path("fixtures/benchmark") + assert fixture_summary_markdown(benchmark_root, summarize_fixtures(benchmark_root)) == ( + ROOT / "docs" / "benchmark" / "0.1.3.md" + ).read_text(encoding="utf-8") + + workflow = yaml.safe_load( + (STARTER / ".github" / "workflows" / "skillgate-review.yml").read_text() + ) + steps = workflow["jobs"]["review"]["steps"] + artifact = next(step for step in steps if step.get("name") == "Upload review artifacts") + publish = next( + step + for step in steps + if step.get("name") == "Publish SARIF on protected branches and manual runs" + ) + assert artifact["if"] == "always()" + assert "skillgate.sarif" in artifact["with"]["path"] + assert publish["if"] == "github.event_name != 'pull_request' && always()" + + +def test_docs_make_external_paths_explicit() -> None: + readme = (ROOT / "README.md").read_text(encoding="utf-8") + starter = (STARTER / "README.md").read_text(encoding="utf-8") + assert "The default path is local and upload-free" in readme + assert "Nothing in a local SkillGate invocation uploads findings automatically." in readme + assert "Connections are explicit opt-ins" in readme + assert "This local command is the default starter path." in starter + assert "workflow in `.github/workflows/skillgate-review.yml` is optional" in starter + assert "local commands above." in starter diff --git a/tests/test_fixtures_docs_release.py b/tests/test_fixtures_docs_release.py index e679fd4..4ebf5e8 100644 --- a/tests/test_fixtures_docs_release.py +++ b/tests/test_fixtures_docs_release.py @@ -80,6 +80,20 @@ def test_cli_fixtures_summary_json() -> None: assert all(item["status"] == "pass" for item in data["fixtures"]) +def test_cli_fixtures_summary_markdown_is_reviewable() -> None: + result = runner.invoke( + app, + ["fixtures", "summary", str(FIXTURES), "--format", "markdown"], + ) + assert result.exit_code == 0 + assert "# SkillGate Benchmark Report" in result.output + assert "Scanner version:" in result.output + assert "## Rule Coverage" in result.output + assert "Expected fixtures" in result.output + assert "## Attribution" in result.output + assert "not a real-world detection accuracy benchmark" in result.output + + def test_cli_provenance_create_and_verify() -> None: workdir = clean_test_dir("provenance-create-verify") (workdir / "SKILL.md").write_text("Safe\n", encoding="utf-8") @@ -364,7 +378,8 @@ def test_docs_are_main_branch_and_discovery_friendly() -> None: assert "actions/upload-artifact@v4" not in action_examples assert "actions/upload-artifact@v6" not in action_examples assert "actions/download-artifact@v4" not in action_examples - assert action_examples.count("if: always()") == 9 + assert action_examples.count("if: always()") == 4 + assert action_examples.count("if: github.event_name != 'pull_request' && always()") == 5 assert 'fail-on-drift: "true"' in action_examples assert workflow["jobs"]["skillgate"]["env"]["FORCE_JAVASCRIPT_ACTIONS_TO_NODE24"] is True assert "actions/upload-artifact@v7" in workflow_text diff --git a/tests/test_preinstall_cli.py b/tests/test_preinstall_cli.py new file mode 100644 index 0000000..ed6f86e --- /dev/null +++ b/tests/test_preinstall_cli.py @@ -0,0 +1,75 @@ +from __future__ import annotations + +import json + +from conftest import ROOT, runner + +from skillgate.cli import app +from skillgate.demo import build_demo_mcpb + +SKILLS_FIXTURES = ROOT / "fixtures" / "skills-validation" + + +def test_preinstall_local_review_validates_discovered_skills() -> None: + result = runner.invoke( + app, + [ + "review", + "preinstall", + str(SKILLS_FIXTURES / "valid-complex"), + "--format", + "json", + ], + ) + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["source"]["kind"] == "local" + assert payload["skills"]["validated"] is True + assert payload["skills"]["summary"]["skills"] == 1 + assert payload["reviewer"]["no_execution"] is True + + +def test_preinstall_fail_on_includes_skill_findings_and_writes_sidecar(tmp_path) -> None: + json_output = tmp_path / "review.json" + result = runner.invoke( + app, + [ + "review", + "preinstall", + str(SKILLS_FIXTURES / "missing-required"), + "--fail-on", + "high", + "--json-output", + str(json_output), + ], + ) + assert result.exit_code == 1 + assert "Review threshold failed" in result.output + assert json.loads(json_output.read_text(encoding="utf-8"))["findings"]["total"] >= 1 + + +def test_preinstall_mcpb_review_uses_bundle_metadata(tmp_path) -> None: + bundle = tmp_path / "reviewable.mcpb" + build_demo_mcpb(bundle) + result = runner.invoke(app, ["review", "preinstall", str(bundle), "--format", "json"]) + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["source"]["kind"] == "mcpb" + assert payload["source"]["digest"] + assert payload["source"]["metadata"]["manifest"]["entry_point"] == "server/index.js" + + +def test_preinstall_invalid_source_exits_two() -> None: + result = runner.invoke(app, ["review", "preinstall", "missing-source"]) + assert result.exit_code == 2 + assert "source does not exist" in result.output + + +def test_preinstall_accepts_a_non_skill_local_file(tmp_path) -> None: + source = tmp_path / "instructions.md" + source.write_text("Review this text without executing it.\n", encoding="utf-8") + result = runner.invoke(app, ["review", "preinstall", str(source), "--format", "json"]) + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["source"]["kind"] == "local" + assert payload["skills"]["validated"] is False diff --git a/tests/test_preinstall_packet.py b/tests/test_preinstall_packet.py new file mode 100644 index 0000000..d09d166 --- /dev/null +++ b/tests/test_preinstall_packet.py @@ -0,0 +1,62 @@ +from __future__ import annotations + +import json +from pathlib import Path + +from conftest import FIXTURES + +from skillgate.preinstall import ( + build_preinstall_packet, + preinstall_packet_json, + render_preinstall_markdown, +) +from skillgate.scan import scan_repository + +SNAPSHOTS = Path(__file__).parent / "snapshots" + + +def packet() -> dict[str, object]: + root = FIXTURES / "05-remote-download-execute" + return build_preinstall_packet( + { + "kind": "local", + "reference": str(root), + "path": str(root), + "metadata": {"token": "super-secret-value", "owner_path": str(root / "private")}, + }, + scan_repository(root), + ) + + +def test_preinstall_packet_is_stable_and_redacted() -> None: + first = packet() + second = packet() + assert first == second + encoded = preinstall_packet_json(first) + assert json.loads(encoded) == first + assert "super-secret-value" not in encoded + assert str(FIXTURES) not in encoded + assert first["schema_version"] == "1" + assert first["reviewer"]["no_execution"] is True + assert first["findings"]["by_severity"]["high"] >= 1 + + +def test_preinstall_packet_markdown_has_decision_sections() -> None: + markdown = render_preinstall_markdown(packet()) + assert markdown.startswith("# SkillGate Pre-install Review\n") + assert "## Capability Inventory" in markdown + assert "## Findings By Severity" in markdown + assert "## Reviewer Next Actions" in markdown + assert "## Limitations" in markdown + assert "No code was executed by the packet renderer." in markdown + assert str(FIXTURES) not in markdown + + +def test_preinstall_packet_snapshots_are_deterministic() -> None: + built = packet() + assert preinstall_packet_json(built) == (SNAPSHOTS / "preinstall_packet.json").read_text( + encoding="utf-8" + ) + assert render_preinstall_markdown(built) == (SNAPSHOTS / "preinstall_packet.txt").read_text( + encoding="utf-8" + ) diff --git a/tools/__init__.py b/tools/__init__.py new file mode 100644 index 0000000..fadab14 --- /dev/null +++ b/tools/__init__.py @@ -0,0 +1 @@ +"""Repository maintenance tools used by tests and release checks.""" diff --git a/uv.lock b/uv.lock index b028d31..7a84f01 100644 --- a/uv.lock +++ b/uv.lock @@ -402,30 +402,27 @@ dependencies = [ { name = "typer" }, ] -[package.optional-dependencies] +[package.dev-dependencies] dev = [ { name = "pytest" }, { name = "ruff" }, -] - -[package.dev-dependencies] -dev = [ { name = "twine" }, ] [package.metadata] requires-dist = [ { name = "pydantic", specifier = ">=2.7" }, - { name = "pytest", marker = "extra == 'dev'", specifier = ">=8.0" }, { name = "pyyaml", specifier = ">=6.0" }, { name = "rich", specifier = ">=13.7" }, - { name = "ruff", marker = "extra == 'dev'", specifier = ">=0.5" }, { name = "typer", specifier = ">=0.12" }, ] -provides-extras = ["dev"] [package.metadata.requires-dev] -dev = [{ name = "twine", specifier = ">=6.2.0" }] +dev = [ + { name = "pytest", specifier = ">=8.0" }, + { name = "ruff", specifier = ">=0.5" }, + { name = "twine", specifier = ">=6.2.0" }, +] [[package]] name = "packaging"