feat(scanner): expose LLM token usage in ScanResult JSON output - #146
Merged
vineethsai7 merged 19 commits intoAug 3, 2026
Merged
Conversation
…tic 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
…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.
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.
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.
…o-ai-defense#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.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change adds provider-normalized LLM token tracking, aggregates usage across analyzers and meta-analysis, exposes it through ChangesLLM usage reporting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SkillScanner
participant LLMAnalyzer
participant MetaAnalyzer
participant ScanResult
SkillScanner->>LLMAnalyzer: analyze_async
LLMAnalyzer-->>SkillScanner: cumulative llm_usage
SkillScanner->>ScanResult: attach analyzer usage
SkillScanner->>MetaAnalyzer: analyze_with_findings
MetaAnalyzer-->>SkillScanner: meta findings and llm_usage
SkillScanner->>ScanResult: merge meta-analyzer usage
ScanResult-->>SkillScanner: serialized result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/output-formats.md`:
- Around line 106-114: Update the JSON example’s analyzers_used list to include
llm_analyzer and/or meta_analyzer so it matches the shown non-empty llm_usage
object. Keep the existing usage values and static analyzer entries unchanged.
In `@skill_scanner/api/router.py`:
- Around line 465-466: Update the ScanResponse model to define an optional
llm_usage field, then pass llm_usage=result.llm_usage when constructing the
single-scan response after merge_meta_analyzer_usage updates result. Preserve
existing response fields and behavior when usage is unavailable.
In `@skill_scanner/core/scanner.py`:
- Line 345: Expose the scanner’s computed llm_usage in the single-skill API
flow: add an optional llm_usage field to ScanResponse, update the manual
ScanResponse construction in the relevant router handler to map
result.llm_usage, and add a regression test verifying the field is returned to
API clients.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a6839f3-14b8-477e-97a3-4f4143302954
📒 Files selected for processing (13)
docs/reference/output-formats.mdskill_scanner/api/router.pyskill_scanner/cli/cli.pyskill_scanner/core/analyzers/llm_analyzer.pyskill_scanner/core/analyzers/llm_request_handler.pyskill_scanner/core/analyzers/meta_analyzer.pyskill_scanner/core/models.pyskill_scanner/core/scanner.pytests/test_cli_llm_usage.pytests/test_llm_analyzer.pytests/test_llm_request_handler.pytests/test_meta_analyzer.pytests/test_models.py
- _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 cisco-ai-defense#144.
- 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>
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
# Conflicts: # tests/test_llm_analyzer.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
Problem
LiteLLM and the Google GenAI SDK both return prompt/completion token counts on every response, but
skill-scannerdiscarded them right after extracting the text content — nothing in the JSON scan output told a caller how many tokens an LLM-as-a-judge scan actually consumed. Downstream pipelines that need to attribute LLM call cost per scan had no option but to monkey-patch LiteLLM internals or proxy HTTP traffic.Solution
This PR adds an additive
llm_usagefield toScanResult.to_dict()—{input_tokens, output_tokens, total_tokens}— aggregated across every LLM call a scan makes (per-file analyzer, consensus-judging runs, and meta-analysis, including its follow-up pass), and omitted entirely when no LLM call was made.Type of Change
Related Issues
Closes #136
Changes Made
llm_request_handler.py: extract token usage at the provider-response boundary for both the LiteLLM path (_extract_token_usage) and the Google GenAI SDK path (_extract_google_sdk_token_usage), normalized to the same{input_tokens, output_tokens, total_tokens}shape regardless of provider. Exposed viaLLMRequestHandler.last_usage, reset at the top of everymake_request()call.llm_analyzer.py/meta_analyzer.py: each analyzer accumulates usage across every LLM call it makes in one run (LLMAnalyzer's consensus-judging loop can call out N times per skill;MetaAnalyzer's follow-up "cover remaining findings" pass is a second call), exposed via allm_usageproperty, reset at the start ofanalyze()/analyze_with_findings().scanner.py: aggregatesllm_usageacross all LLM analyzers that ran in a scan; only set onScanResultwhen at least one analyzer actually reported nonzero usage.meta_analyzer.py: addsmerge_meta_analyzer_usage().MetaAnalyzeralways runs as a separate post-processing step afterSkillScannerhas already built theScanResult(seecli.py/api/router.py), so its token spend can't be seen by the scanner's own aggregation — this folds it intoresult.llm_usageafterward. Wired into all 5 call sites where meta-analysis runs post-scan (cli.py:scan_command,scan_all_command,scan_repo_command;api/router.py: single + batch scan).models.py: new optionalScanResult.llm_usage: dict[str, int] | Nonefield, included into_dict()only when truthy.docs/reference/output-formats.md: documents the new field and its omission behavior.Testing
Test Coverage
144 new/updated tests across
test_llm_request_handler.py,test_llm_analyzer.py,test_meta_analyzer.py,test_models.py, and a newtest_cli_llm_usage.py. That last file drives the realcli.scan_command()/scan_all_command()entry points end-to-end (mocked provider responses, no internal helpers called directly) specifically to guard againstllm_usagesilently regressing to counting only the per-file analyzer and dropping meta-analysis spend.Manual Testing
Also ran the real CLI entry point (
cli.scan_command()) against a fixture skill with mocked LLM (1,234/321 tokens) and meta-analyzer (4,321/654 tokens) responses and inspected the actual JSON output:Expected:
5555 = 1234 + 4321and975 = 321 + 654(both analyzers' spend summed correctly); field absent entirely on a static-only scan (--use-llm/--enable-metaoff).Actual: matched exactly on both counts.
Checklist
Code Quality
None-safe for providers that omit usage)Documentation
docs/reference/output-formats.mdCHANGELOG.mdin this repoSecurity
Testing
uv run pre-commit run --all-filesuv run python evals/runners/benchmark_runner.pyPerformance Impact
Additional Notes
One known minor gap, left for a follow-up rather than blocking this PR: if
MetaAnalyzer's LLM call succeeds but something after it (e.g.apply_meta_analysis_to_results()) throws, those tokens are silently dropped fromllm_usagesince the merge only happens on the success path.MetaAnalyzer.analyze_with_findings()already catches its own provider-level errors internally, so this is only reachable in a narrow post-call failure case.Summary by CodeRabbit
New Features
Documentation
llm_usagefield and availability guidance.Tests