Commit 05e5e37
feat(scanner): expose LLM token usage in ScanResult JSON output (#146)
* feat(adjudicator): demote literal-regex false positives on deterministic HIGH+ findings
Introduces an optional per-finding adjudicator that runs between the
deterministic analyzers (static / pipeline / behavioral / bytecode / yara)
and the LLM analyzer. For each deterministic HIGH or CRITICAL finding it
asks the configured 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 identified as literal-regex false
positives (verdict=false_positive, confidence>=threshold) are demoted to
INFO for downstream verdict computation. The original severity and the
LLM's reasoning are preserved in the finding's metadata for audit.
Because the adjudicator runs before the LLM analyzer, demoted findings do
not enter the LLM analyzer's static-finding enrichment context — so a
wrong deterministic HIGH cannot be amplified into further LLM findings
citing the same pattern hit. This addresses the confirmation-cascade
failure mode described in the linked issue without any changes to
llm_analyzer.py.
Safety property (load-bearing): the adjudicator can only demote findings,
never promote them. LLM errors, timeouts, malformed output, or unexpected
verdicts all leave the finding at its original severity. Enabling this
pass cannot introduce false negatives.
Off by default (--adjudicate CLI flag, ScanPolicy.adjudicator.enabled).
Reuses the same env-var conventions as the LLM analyzer for model and
temperature configuration, so Claude 4.x on Bedrock and OpenAI o1-series
work out of the box.
Includes:
- New skill_scanner/core/analyzers/adjudicator.py (self-contained module)
- AdjudicatorPolicy in ScanPolicy
- --adjudicate CLI flag on both scan entry points
- Integration in scanner.py between Phase 1 and Phase 2 analyzers
- Audit records surfaced in ScanResult.scan_metadata.adjudicator
- 11 pytest tests covering the load-bearing safety cases
- docs/architecture/analyzers/adjudicator.md
* style(adjudicator): ruff --fix + ruff format for pre-commit compliance
* fix(adjudicator): address CodeRabbit review — path containment, YAML round-trip, override precedence, prompt hardening
Six review-driven fixes plus a soften-the-language docs pass.
1. **Path containment** (adjudicator.py:_adjudicate_one) — resolve
``skill_dir / file_path`` and reject anything that lands outside the
skill directory. Absolute paths and ``..`` traversal now short-circuit
to a "skipped" AdjudicationResult before any file is read.
2. **YAML round-trip for AdjudicatorPolicy** (scan_policy.py:_from_dict
and _to_dict) — custom policy files with an ``adjudicator:`` section
were silently discarded on load and never emitted on ``to_yaml``.
Wired through following the AnalyzersPolicy / LLMAnalysisPolicy
pattern already in the file.
3. **--adjudicate on scan-repo** (cli.py:scan_repo_command) — the flag
was defined in _add_common_scan_flags but scan_repo_command didn't
read it, so ``skill-scanner scan-repo <url> --adjudicate`` silently
did nothing. Mirrors the toggle already present on scan_command and
scan_all_command.
4. **Confidence range enforcement** (adjudicator.py:_adjudicate_one) —
the response contract is confidence 1-5, but the code accepted any
integer. A malformed ``{"verdict":"false_positive","confidence":999}``
would demote past any min_fp_confidence threshold. Now validated and
fails closed on out-of-range values (including 0).
5. **Preserve adjudicator demotion through severity_overrides**
(scanner.py:_apply_severity_overrides) — a policy severity-override
entry could raise a demoted finding back to HIGH/CRITICAL, defeating
the demote-to-INFO contract. _apply_severity_overrides now skips
findings that carry ``metadata['adjudication']['demoted_to']``.
6. **Prompt-injection hardening** (adjudicator.py:_call_llm and new
_SYSTEM_PROMPT constant) — scanned file content is untrusted evidence.
Split the request into a role=system rubric (trusted) and a role=user
payload (rubric + untrusted evidence), with the system prompt
explicitly instructing the model to ignore any instructions embedded
in the file content. Defense in depth: the demote-only invariant
already bounds the blast radius, but keeping trusted rubric separate
from untrusted evidence is standard practice for LLM-as-judge flows.
7. **Docs / docstring softening** — replaced "cannot introduce false
negatives" with a more accurate statement: error paths (LLM
unavailable, malformed output, out-of-range confidence, path escape)
preserve severity, but a wrong ``false_positive`` verdict from the
LLM itself can still demote a real threat. That's why the pass is
off by default and every demotion is preserved in metadata for review.
New tests (all 16 pass):
- test_out_of_range_confidence_keeps_original_severity
- test_zero_confidence_keeps_original_severity
- test_absolute_path_outside_skill_dir_is_skipped
- test_parent_traversal_is_skipped
- test_llm_receives_system_prompt
Ruff-clean, format-clean.
* feat: add trusted_reference_domains to LLM analysis policy
Add a policy-driven mechanism to demote LLM findings (transitive trust,
supply chain) to LOW when all referenced domains are declared as trusted
in the scan policy.
This addresses systematic false positives for organizations hosting
skills and documentation on self-managed GitLab/GitHub instances, where
the LLM analyzer flags references to internal repos as external threats.
Changes:
- LLMAnalysisPolicy: new trusted_reference_domains field (set[str])
- llm_analyzer._convert_to_findings: demotion logic after existing
is_internal_file_reading filter
- Two helpers: _references_only_trusted_domains (URL extraction) and
_mentions_only_trusted_domains (plain-text domain detection)
- default_policy.yaml: empty default list with documentation
The mechanism mirrors known_installer_domains: findings are demoted to
LOW (maintaining visibility) rather than suppressed entirely.
* fix(llm): allow Vertex AI to use ambient Application Default Credentials
ProviderConfig.validate() required a truthy credential for every provider
except Bedrock and Ollama, and the only credential source it checked for
Vertex was GOOGLE_APPLICATION_CREDENTIALS. This blocked ambient auth via
a GCE/Cloud Run attached service account or Workload Identity, even though
LiteLLM/google-auth already fall back to it automatically when no explicit
credential is passed -- the same pattern already supported for Bedrock's
IAM role. Excludes is_vertex from the check, mirroring the Bedrock/Ollama
precedent, and documents the fallback.
* Bumps to resolve security findings
* feat(scanner): expose LLM token usage in ScanResult JSON output (#136)
LiteLLM and the Google GenAI SDK both return prompt/completion token
counts on every response, but the scanner discarded them after pulling
out the text content, leaving downstream pipelines with no way to
attribute LLM call cost to a scan without monkey-patching provider
internals.
Extract token usage at the provider-response boundary for both LiteLLM
and Google SDK request paths, accumulate it per analyzer (covering
multi-call cases like consensus-judging runs and meta-analysis
follow-up passes), aggregate across all LLM analyzers in
SkillScanner, and fold in MetaAnalyzer's spend separately since it
runs as a post-processing step outside the scanner's own aggregation.
Surfaces as an additive `llm_usage` field on ScanResult.to_dict(),
omitted entirely on static-only scans.
* fix(llm): don't leak Vertex ADC credential path into GEMINI_API_KEY
- _resolve_api_key() now returns None for Vertex instead of the
GOOGLE_APPLICATION_CREDENTIALS path, since vertex_ai/gemini-* models
set both is_vertex and is_gemini, which was causing the file path to
be written into GEMINI_API_KEY.
- Regenerated configuration-reference.md via generate_reference_docs.py
instead of hand-editing, and updated the underlying descriptions so
the doc doesn't drift on next regeneration.
Addresses CodeRabbit review feedbak on #144.
* revert unrelated cli-command-reference.md regeneration
* fix: address CodeRabbit review comments on PR #146
- Fix inconsistent JSON example: analyzers_used now includes
llm_analyzer/meta_analyzer to match the shown non-empty llm_usage block.
- Expose ScanResult.llm_usage through the /scan API response: ScanResponse
was missing the field, so single-scan (and /scan-upload, which delegates
to it) API clients never received token usage even though the scanner
computed it. Batch scan was unaffected since it serializes via to_dict().
- Add regression tests covering both the combined analyzer+meta-analyzer
usage case and the omitted-when-disabled case.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(llm): scope trusted-domain severity demotion
---------
Co-authored-by: Rohan Isawe <rohan.isawe@smartsheet.com>
Co-authored-by: Olivier Jacques <ojacques2@gmail.com>
Co-authored-by: gyrospectre <7224858+gyrospectre@users.noreply.github.com>
Co-authored-by: Vineeth Sai Narajala <vnarajal@cisco.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>1 parent d76b6c1 commit 05e5e37
14 files changed
Lines changed: 779 additions & 8 deletions
File tree
- docs/reference
- skill_scanner
- api
- cli
- core
- analyzers
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
95 | 95 | | |
96 | 96 | | |
97 | 97 | | |
98 | | - | |
| 98 | + | |
99 | 99 | | |
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
105 | 110 | | |
106 | 111 | | |
107 | 112 | | |
108 | 113 | | |
| 114 | + | |
| 115 | + | |
109 | 116 | | |
110 | 117 | | |
111 | 118 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
97 | 97 | | |
98 | 98 | | |
99 | 99 | | |
| 100 | + | |
100 | 101 | | |
101 | | - | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
102 | 107 | | |
103 | 108 | | |
104 | 109 | | |
105 | 110 | | |
106 | 111 | | |
107 | 112 | | |
| 113 | + | |
108 | 114 | | |
109 | 115 | | |
110 | 116 | | |
| |||
215 | 221 | | |
216 | 222 | | |
217 | 223 | | |
| 224 | + | |
218 | 225 | | |
219 | 226 | | |
220 | 227 | | |
| |||
456 | 463 | | |
457 | 464 | | |
458 | 465 | | |
| 466 | + | |
| 467 | + | |
459 | 468 | | |
460 | 469 | | |
461 | 470 | | |
| |||
469 | 478 | | |
470 | 479 | | |
471 | 480 | | |
| 481 | + | |
472 | 482 | | |
473 | 483 | | |
474 | 484 | | |
| |||
712 | 722 | | |
713 | 723 | | |
714 | 724 | | |
| 725 | + | |
| 726 | + | |
715 | 727 | | |
716 | 728 | | |
717 | 729 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
| 57 | + | |
57 | 58 | | |
58 | | - | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
59 | 64 | | |
60 | 65 | | |
61 | 66 | | |
62 | 67 | | |
63 | 68 | | |
64 | 69 | | |
| 70 | + | |
65 | 71 | | |
66 | 72 | | |
67 | 73 | | |
| |||
411 | 417 | | |
412 | 418 | | |
413 | 419 | | |
| 420 | + | |
| 421 | + | |
414 | 422 | | |
415 | 423 | | |
416 | 424 | | |
| |||
524 | 532 | | |
525 | 533 | | |
526 | 534 | | |
| 535 | + | |
| 536 | + | |
527 | 537 | | |
528 | 538 | | |
529 | 539 | | |
| |||
649 | 659 | | |
650 | 660 | | |
651 | 661 | | |
| 662 | + | |
| 663 | + | |
652 | 664 | | |
653 | 665 | | |
654 | 666 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
41 | 41 | | |
42 | 42 | | |
43 | 43 | | |
44 | | - | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
45 | 51 | | |
46 | 52 | | |
47 | 53 | | |
| |||
254 | 260 | | |
255 | 261 | | |
256 | 262 | | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
257 | 266 | | |
258 | 267 | | |
259 | 268 | | |
| |||
263 | 272 | | |
264 | 273 | | |
265 | 274 | | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
266 | 280 | | |
267 | 281 | | |
268 | 282 | | |
| |||
327 | 341 | | |
328 | 342 | | |
329 | 343 | | |
| 344 | + | |
330 | 345 | | |
331 | 346 | | |
332 | 347 | | |
| |||
450 | 465 | | |
451 | 466 | | |
452 | 467 | | |
| 468 | + | |
453 | 469 | | |
454 | 470 | | |
455 | 471 | | |
| |||
503 | 519 | | |
504 | 520 | | |
505 | 521 | | |
| 522 | + | |
506 | 523 | | |
507 | 524 | | |
508 | 525 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
31 | | - | |
| 31 | + | |
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
35 | 88 | | |
36 | 89 | | |
37 | 90 | | |
| |||
151 | 204 | | |
152 | 205 | | |
153 | 206 | | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
154 | 215 | | |
155 | 216 | | |
156 | 217 | | |
| |||
283 | 344 | | |
284 | 345 | | |
285 | 346 | | |
| 347 | + | |
286 | 348 | | |
287 | 349 | | |
288 | 350 | | |
| |||
322 | 384 | | |
323 | 385 | | |
324 | 386 | | |
| 387 | + | |
325 | 388 | | |
326 | 389 | | |
327 | 390 | | |
| |||
336 | 399 | | |
337 | 400 | | |
338 | 401 | | |
| 402 | + | |
339 | 403 | | |
340 | 404 | | |
341 | 405 | | |
| |||
407 | 471 | | |
408 | 472 | | |
409 | 473 | | |
| 474 | + | |
410 | 475 | | |
411 | 476 | | |
412 | 477 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
48 | | - | |
| 48 | + | |
49 | 49 | | |
50 | 50 | | |
51 | | - | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
52 | 60 | | |
53 | 61 | | |
54 | 62 | | |
| |||
365 | 373 | | |
366 | 374 | | |
367 | 375 | | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
368 | 379 | | |
369 | 380 | | |
370 | 381 | | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
371 | 387 | | |
372 | 388 | | |
373 | 389 | | |
| |||
427 | 443 | | |
428 | 444 | | |
429 | 445 | | |
| 446 | + | |
| 447 | + | |
430 | 448 | | |
431 | 449 | | |
432 | 450 | | |
| |||
877 | 895 | | |
878 | 896 | | |
879 | 897 | | |
| 898 | + | |
880 | 899 | | |
881 | 900 | | |
882 | 901 | | |
| |||
1128 | 1147 | | |
1129 | 1148 | | |
1130 | 1149 | | |
| 1150 | + | |
| 1151 | + | |
| 1152 | + | |
| 1153 | + | |
| 1154 | + | |
| 1155 | + | |
| 1156 | + | |
| 1157 | + | |
| 1158 | + | |
| 1159 | + | |
| 1160 | + | |
| 1161 | + | |
| 1162 | + | |
| 1163 | + | |
| 1164 | + | |
| 1165 | + | |
| 1166 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
218 | 218 | | |
219 | 219 | | |
220 | 220 | | |
| 221 | + | |
221 | 222 | | |
222 | 223 | | |
223 | 224 | | |
| |||
265 | 266 | | |
266 | 267 | | |
267 | 268 | | |
| 269 | + | |
| 270 | + | |
268 | 271 | | |
269 | 272 | | |
270 | 273 | | |
| |||
0 commit comments