Skip to content

Commit ea9eb7e

Browse files
committed
Merge main into feat/pr2-unpinned-deps
2 parents 62c10be + 5dca0c5 commit ea9eb7e

55 files changed

Lines changed: 17154 additions & 6871 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.env.example

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@
3131

3232
# Google Vertex AI (service account - no API key needed)
3333
# GOOGLE_APPLICATION_CREDENTIALS=/path/to/service-account.json
34-
# VERTEXAI_PROJECT=my-gcp-project
35-
# VERTEXAI_LOCATION=us-central1
3634
# SKILL_SCANNER_LLM_MODEL=vertex_ai/gemini-1.5-pro
3735

3836
# Meta-Analyzer Configuration (optional)

.github/workflows/release.yml

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ jobs:
2424
permissions:
2525
id-token: write
2626
contents: write # needed to create/update the GitHub release assets
27+
attestations: write # needed to store build provenance and SBOM attestations
2728
steps:
2829
- name: Checkout code
2930
uses: actions/checkout@v4
@@ -53,6 +54,38 @@ jobs:
5354
--no-emit-project \
5455
--output-file release-assets/requirements.txt
5556
57+
# SBOM is generated from the frozen export above so all release artifacts
58+
# describe one locked tree. The project version is dynamic (VCS-derived),
59+
# so it is stamped from the workflow input.
60+
- name: Generate CycloneDX SBOM
61+
env:
62+
VERSION: ${{ inputs.version }}
63+
run: |
64+
uv sync --frozen --only-group sbom
65+
uv run --no-sync cyclonedx-py requirements \
66+
release-assets/requirements.txt \
67+
--pyproject pyproject.toml \
68+
--output-reproducible \
69+
--output-file release-assets/sbom.cdx.json
70+
jq --arg v "$VERSION" \
71+
'.metadata.component.version = $v
72+
| .metadata.component.purl = "pkg:pypi/cisco-ai-skill-scanner@\($v)"' \
73+
release-assets/sbom.cdx.json > sbom.tmp
74+
mv sbom.tmp release-assets/sbom.cdx.json
75+
76+
- name: Attest build provenance
77+
uses: actions/attest-build-provenance@0f67c3f4856b2e3261c31976d6725780e5e4c373 # v4.1.1
78+
with:
79+
subject-path: |
80+
dist/*
81+
release-assets/*
82+
83+
- name: Attest SBOM
84+
uses: actions/attest-sbom@c604332985a26aa8cf1bdc465b92731239ec6b9e # v4.1.0
85+
with:
86+
subject-path: dist/*
87+
sbom-path: release-assets/sbom.cdx.json
88+
5689
- name: Check GitHub release state
5790
id: release-state
5891
env:
@@ -66,7 +99,7 @@ jobs:
6699
67100
if [[ "$is_draft" == "false" ]]; then
68101
echo "published=true" >> "$GITHUB_OUTPUT"
69-
echo "::warning::GitHub release $VERSION is already published. Immutable releases cannot accept new assets, so requirements.txt upload will be skipped."
102+
echo "::warning::GitHub release $VERSION is already published. Immutable releases cannot accept new assets, so release asset upload will be skipped."
70103
else
71104
echo "published=false" >> "$GITHUB_OUTPUT"
72105
fi
@@ -76,12 +109,14 @@ jobs:
76109
echo "published=false" >> "$GITHUB_OUTPUT"
77110
fi
78111
79-
- name: Attach requirements.txt to draft GitHub release
112+
- name: Attach requirements.txt and SBOM to draft GitHub release
80113
if: steps.release-state.outputs.published != 'true'
81114
uses: softprops/action-gh-release@v2
82115
with:
83116
tag_name: ${{ inputs.version }}
84-
files: release-assets/requirements.txt
117+
files: |
118+
release-assets/requirements.txt
119+
release-assets/sbom.cdx.json
85120
fail_on_unmatched_files: true
86121
draft: true
87122

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,4 @@ scripts/ui_security_verify.py
108108

109109
# User-generated scan policy (default TUI output)
110110
scan_policy.yaml
111+
ISSUE_DRAFT.md

.pre-commit-hooks.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
description: Scan agent skill packages for security vulnerabilities
44
entry: skill-scanner-pre-commit
55
language: python
6-
types: [file]
76
pass_filenames: false
7+
require_serial: true
8+
always_run: true
89
stages: [pre-commit]

README.md

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,10 +338,26 @@ repos:
338338
Or install the built-in hook directly:
339339
340340
```bash
341-
skill-scanner-pre-commit install
341+
skill-scanner-pre-commit --install
342342
```
343343

344-
The hook automatically detects which skill directories have staged changes and only scans those, keeping commit times fast. Use `--all` to scan everything.
344+
The hook maps changed files to their nearest `SKILL.md` and scans each affected
345+
skill once. During a normal commit, it reads the staged diff. In CI, compare two
346+
revisions so no staged index is required:
347+
348+
```bash
349+
pre-commit run skill-scanner --from-ref "$BASE_SHA" --to-ref "$HEAD_SHA"
350+
```
351+
352+
Both revisions must exist in the checkout. To scan every configured skill,
353+
invoke the hook directly:
354+
355+
```bash
356+
skill-scanner-pre-commit --scan-all
357+
```
358+
359+
Alternatively, configure `args: [--scan-all]` for the hook in
360+
`.pre-commit-config.yaml`.
345361

346362
---
347363

SECURITY.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,18 @@ following steps:
6565
who install with `pip` can reproduce the exact same dependency tree we ship
6666
and tested against, with hash verification.
6767
- **PyPI Trusted Publishers.** Releases are uploaded to PyPI via OIDC; no
68-
long-lived API tokens are stored in the repository or in CI secrets.
68+
long-lived API tokens are stored in the repository or in CI secrets. Uploads
69+
carry [PEP 740](https://peps.python.org/pep-0740/) attestations generated
70+
during publish.
71+
- **SLSA build provenance.** Every release run attests the wheel, sdist, and
72+
release assets with
73+
[`actions/attest-build-provenance`](https://github.com/actions/attest-build-provenance).
74+
Verify any artifact with
75+
`gh attestation verify <artifact> --repo cisco-ai-defense/skill-scanner`.
76+
- **CycloneDX SBOM.** Each GitHub release includes an `sbom.cdx.json` generated
77+
from the same frozen export as `requirements.txt` and bound to the
78+
distributions with
79+
[`actions/attest-sbom`](https://github.com/actions/attest-sbom).
6980
- **Loose abstract constraints in `pyproject.toml`.** Library consumers can
7081
resolve transitive security patches forward without forced cascades. See
7182
[`CONTRIBUTING.md` § Dependency Policy](/CONTRIBUTING.md#dependency-policy)
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
# Adjudicator
2+
3+
The Adjudicator is an optional pass that runs **between the deterministic analyzers and the LLM analyzer**. For each deterministic HIGH/CRITICAL finding it asks an LLM whether the file around the matched line actually contains the threat the rule was designed to catch, or whether the regex fired on benign content. Findings the LLM identifies as literal-regex false positives are demoted to `INFO` for downstream verdict computation.
4+
5+
## Overview
6+
7+
When enabled via the `--adjudicate` CLI flag (or `ScanPolicy.adjudicator.enabled = True` on the API), the Adjudicator performs:
8+
9+
- **False positive demotion**: Uses an LLM to reason about whether a deterministic HIGH/CRITICAL match represents a real instance of the threat, or a coincidental regex hit on benign prose.
10+
- **Cascade prevention**: Because it runs before the LLM analyzer, demoted findings never enter the LLM analyzer's static-finding enrichment context — so a wrong deterministic HIGH cannot be amplified into LLM findings citing the same pattern hit.
11+
- **Audit trail**: Every finding it considers is recorded in `scan_metadata.adjudicator.audit` with the LLM's verdict, confidence, and reason.
12+
13+
The adjudicator only touches deterministic findings (static, pipeline, behavioral, bytecode, yara analyzers) at HIGH or CRITICAL severity. LLM and other advisory findings are outside its scope.
14+
15+
## Safety property
16+
17+
**The Adjudicator is demote-only.** It can lower a finding's severity to `INFO`, and can never raise a finding's severity.
18+
19+
The demote-only constraint bounds the failure surface. On **error paths** — LLM unavailable, timeout, malformed JSON, unexpected verdict, out-of-range confidence, path-escape attempt — the finding stays at its original severity and no metadata is written. Those specific paths cannot introduce false negatives.
20+
21+
**A wrong `false_positive` verdict from the LLM itself can still demote a real threat.** That is a genuine failure mode and is why the pass is off by default, why the confidence threshold is configurable, and why every demotion is preserved in `finding.metadata["adjudication"]` for review and override. Adversarial content in the scanned skill can also attempt to bias the adjudicator (see the system-prompt hardening in the implementation) — again, the worst case is a real finding demoted to `INFO`, but this is a real (not zero-probability) risk that operators should weigh when enabling the pass.
22+
23+
## How It Works
24+
25+
1. **Deterministic analyzers run first** (static, pipeline, behavioral, bytecode, yara). This is unchanged.
26+
2. **Adjudicator inspects HIGH/CRITICAL findings from those analyzers.** For each finding it:
27+
- Extracts the matched line and a wide surrounding context (whole file for files ≤ 600 lines, ± 25 lines otherwise). The wide window makes the adjudicator resilient to the scanner's occasional off-by-N line-number reports on markdown content.
28+
- Pulls the rule's `description`, `category`, and `default_severity` from the rule registry.
29+
- Sends both to the LLM with a fixed prompt asking `real` vs `false_positive`, a 1–5 confidence, and a one-sentence reason.
30+
3. **Demotes on high-confidence FP verdicts.** If the LLM returns `verdict = "false_positive"` and `confidence >= min_fp_confidence` (default 3), the finding's severity is lowered to `INFO`. The original severity is preserved in `finding.metadata["adjudication"]["original_severity"]` for audit.
31+
4. **LLM analyzer runs next** (if enabled). Its `static_findings_summary` enrichment now excludes demoted findings, so the LLM analyzer cannot cross-confirm a false-positive deterministic HIGH.
32+
5. **The rest of the pipeline is unchanged**: severity overrides, disabled rules, analyzability, deduplication, policy fingerprinting.
33+
34+
## CLI Usage
35+
36+
```bash
37+
# Basic — deterministic analyzers only, with adjudicator active
38+
skill-scanner scan /path/to/skill --adjudicate
39+
40+
# Full stack — adjudicator gates the LLM analyzer's static enrichment
41+
skill-scanner scan /path/to/skill --use-llm --adjudicate
42+
43+
# Full stack with meta-analysis
44+
skill-scanner scan /path/to/skill --use-llm --enable-meta --adjudicate
45+
```
46+
47+
**Requirements:**
48+
49+
- An LLM model must be configured via `SKILL_SCANNER_LLM_MODEL` (or `SKILL_SCANNER_ADJUDICATOR_LLM_MODEL` to override for the adjudicator specifically).
50+
- API key via `SKILL_SCANNER_LLM_API_KEY` for providers that need one; AWS credentials for `bedrock/...` models.
51+
- LiteLLM must be installed.
52+
53+
If any of the above are missing, the adjudicator logs a debug message and skips every finding — the scan behaves identically to a run with `--adjudicate` off. This is intentional: unavailability is not an error, it's a no-op.
54+
55+
## Configuration
56+
57+
The Adjudicator is configured via `AdjudicatorPolicy` on the `ScanPolicy`:
58+
59+
```yaml
60+
adjudicator:
61+
enabled: false # master toggle; --adjudicate CLI flag also sets this
62+
min_fp_confidence: 3 # 1-5; LLM confidence required to demote (default 3)
63+
```
64+
65+
Environment variables (in order of precedence):
66+
67+
- `SKILL_SCANNER_ADJUDICATOR_LLM_MODEL` — model override specific to the adjudicator
68+
- `SKILL_SCANNER_ADJUDICATOR_LLM_TEMPERATURE` — temperature override, or `"none"` to omit
69+
- `SKILL_SCANNER_LLM_MODEL` — fallback if the adjudicator-specific var is unset
70+
- `SKILL_SCANNER_LLM_TEMPERATURE` — fallback if the adjudicator-specific var is unset
71+
72+
## Output
73+
74+
Demoted findings appear in the final report with:
75+
76+
- `severity: "INFO"` (the effective severity used for verdict computation)
77+
- `metadata.adjudication`:
78+
- `original_severity`: what it was before demotion (e.g. `"HIGH"`)
79+
- `verdict`: `"false_positive"`
80+
- `confidence`: `1-5`
81+
- `reason`: one-sentence rationale from the LLM
82+
- `demoted_to`: `"INFO"`
83+
- `model_id`: which model made the decision
84+
85+
The scan's `scan_metadata.adjudicator` section summarizes the pass:
86+
87+
```json
88+
{
89+
"adjudicator": {
90+
"considered": 4,
91+
"demoted": 1,
92+
"audit": [
93+
{
94+
"rule_id": "PROMPT_INJECTION_CONCEALMENT",
95+
"verdict": "false_positive",
96+
"confidence": 5,
97+
"reason": "The phrase 'do not notify the user' refers to a routine pre-flight column-update succeeding silently; the write itself is visible.",
98+
"demoted_to": "INFO",
99+
"model_id": "bedrock/converse/anthropic.claude-opus-4-8-20240229-v1:0"
100+
},
101+
...
102+
]
103+
}
104+
}
105+
```
106+
107+
## Cost
108+
109+
Per skill: 0–3 LLM calls × ~200 input tokens + ~50 output tokens ≈ $0.005 on Opus 4.x. Negligible relative to the existing LLM analyzer + meta-analyzer cost.
110+
111+
## Concurrency
112+
113+
The adjudicator uses a module-level lock to serialize its LLM calls across parallel workers. This prevents adjudicator calls from competing with the main LLM analyzer's calls for backend rate limits. Because adjudicator calls are short (~250 output tokens) and rare (0–3 per skill), the serialization overhead is at most a few seconds per scan and eliminates a class of transient 5xx-induced regressions.
114+
115+
## When to enable
116+
117+
- **Yes**: when your policy treats deterministic HIGH+ as auto-reject and false positives on that class of finding are causing real friction for reviewers.
118+
- **Yes**: when you're running the LLM analyzer and observing findings that cite deterministic pattern hits (the "confirmation cascade" failure mode).
119+
- **Maybe**: for CI gating where you accept a small per-scan LLM cost in exchange for fewer benign auto-rejects.
120+
- **No**: for pure deterministic gates that never enable LLM analysis anyway — the adjudicator has nothing to demote that would matter.
121+
122+
## Relation to other analyzers
123+
124+
- **Adjudicator vs. Meta-analyzer**: they solve different problems and can be enabled together. The adjudicator runs *before* the LLM analyzer and demotes deterministic false positives at their source. The meta-analyzer runs *after* all analyzers and re-scores or correlates the full finding set. Both can be on simultaneously; they don't conflict.
125+
- **Adjudicator vs. `--llm-consensus-runs`**: consensus reduces run-to-run flap on LLM findings by voting across N runs of the LLM analyzer. Consensus does not affect deterministic findings and does not address the cross-analyzer confirmation cascade. The adjudicator addresses a different failure mode and is complementary.

docs/development/integrations.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,23 @@ chmod +x .git/hooks/pre-commit
112112

113113
The hook entry point is `skill-scanner-pre-commit`, which is installed alongside the main CLI.
114114

115+
### Incremental checks in CI
116+
117+
There is no staged index to inspect after a CI checkout. Use pre-commit's
118+
revision comparison; the hook reads those revisions from pre-commit's
119+
environment and computes the changed paths internally:
120+
121+
```bash
122+
pre-commit run skill-scanner \
123+
--from-ref "$BASE_SHA" \
124+
--to-ref "$HEAD_SHA"
125+
```
126+
127+
The hook uses a separate deletion diff so removed paths are included, resolves
128+
each changed path to its nearest parent containing `SKILL.md`, and scans each
129+
affected skill once. Both revisions must be present locally; configure the
130+
checkout step to fetch enough history for the selected base and head SHAs.
131+
115132
## Policy-Aware CI
116133

117134
Keep preset strategy explicit by workflow stage:

docs/development/setup-and-testing.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,19 @@ repos:
182182
- id: skill-scanner
183183
```
184184
185-
The hook entry point is `skill-scanner-pre-commit` (defined in `pyproject.toml`). It automatically detects staged skill directories via `git diff --cached` and only scans those.
185+
The hook entry point is `skill-scanner-pre-commit` (defined in `pyproject.toml`).
186+
It computes staged paths internally, maps each file to the nearest parent
187+
containing `SKILL.md`, and scans every affected skill once.
188+
189+
For incremental CI checks, let pre-commit expose the two available revisions;
190+
the hook computes their changed paths, including deletions, internally:
191+
192+
```bash
193+
pre-commit run skill-scanner --from-ref "$BASE_SHA" --to-ref "$HEAD_SHA"
194+
```
195+
196+
Ensure the CI checkout contains both revisions (for example, avoid a shallow
197+
checkout that omits the base commit).
186198

187199
## GitHub Actions Reusable Workflow
188200

docs/features/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ See [API Server](../user-guide/api-server.md) and [API Endpoint Reference](../re
273273
Block risky findings before they reach the repository. The hook scans staged skill changes and fails the commit when findings exceed a severity threshold.
274274

275275
```bash
276-
skill-scanner-pre-commit install
276+
skill-scanner-pre-commit --install
277277
```
278278

279279
<details>

0 commit comments

Comments
 (0)