diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f58b090..22eecbea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,24 @@ +### 2.8.1 (Thursday, August 06, 2026) +### Features/Bug Fixes +* fix(llm): isolate malformed structured responses per batch +--- +### 2.8.0 (Thursday, August 06, 2026) +### Features/Bug Fixes +* fix(baseline): exclude selected baseline from scans +--- +### 2.7.2 (Thursday, August 06, 2026) +### Features/Bug Fixes +* fix(pe3): distinguish OAuth access-token nouns from credential access +--- +### 2.7.0 (Thursday, August 06, 2026) +### Features/Bug Fixes +* fix(telemetry): harden inference usage normalization +--- +### 2.6.0 (Wednesday, August 05, 2026) +### Features/Bug Fixes +* feat(release): auto-generate versioned release notes like CHANGELOG +* feat(telemetry): export provider inference usage +--- ### 2.5.3 (Tuesday, August 04, 2026) ### Features/Bug Fixes * fix(analyzers): share Python AST parsing for environment-read detection (#332) diff --git a/README.md b/README.md index c42ba2e2..2d4d64c1 100644 --- a/README.md +++ b/README.md @@ -215,6 +215,10 @@ A baseline can also use drift-tolerant glob rules (by rule id, file path, or message) — see [`.skillspector-baseline.example.yaml`](.skillspector-baseline.example.yaml). Exact fingerprint baselines are evidence-bound: changing the scanned source or SkillSpector version keeps the finding active until it is reviewed again. +When a selected baseline or baseline output is stored inside the skill +directory, SkillSpector excludes that exact file from content analysis so its +suppression text cannot create findings or enter regenerated fingerprints; +sibling files remain in normal scan scope. ### LLM Analysis @@ -639,13 +643,46 @@ The top-level shape is (this example shows a full LLM-backed scan; with `--no-ll "risk_assessment": { "score": 0, "severity": "LOW", "recommendation": "SAFE" }, "components": [ { "path": "...", "type": "...", "lines": 0, "executable": false, "size_bytes": 0 } ], "issues": [ { "id": "...", "category": "...", "severity": "...", "confidence": 0.0, "location": { "file": "...", "start_line": 0 } } ], - "metadata": { "has_executable_scripts": false, "skillspector_version": "...", "llm_requested": true, "llm_available": true } + "metadata": { + "has_executable_scripts": false, + "skillspector_version": "...", + "llm_requested": true, + "llm_available": true, + "inference_usage": [ + { + "node": "semantic_security_discovery", + "request_kind": "structured_output", + "provider": "anthropic", + "model": "claude-opus-4-6", + "model_source": "provider_response", + "usage_source": "provider_response", + "prompt_tokens": 1000, + "completion_tokens": 100, + "cached_tokens": 400, + "cache_write_tokens": 50, + "total_tokens": 1100 + } + ] + } } ``` - `risk_assessment.severity` ∈ `LOW | MEDIUM | HIGH | CRITICAL`. - `risk_assessment.recommendation` ∈ `SAFE | CAUTION | DO_NOT_INSTALL`, mapped from severity: `LOW → SAFE`, `MEDIUM → CAUTION`, `HIGH`/`CRITICAL → DO_NOT_INSTALL`. - `metadata.llm_error` appears only when LLM analysis was requested but unavailable. +- `metadata.inference_usage` contains one sanitized record per LLM response when the + provider exposes token counters. It is an empty list when usage is unavailable; + SkillSpector never estimates missing tokens. Prompt totals are inclusive of cache + reads and writes so downstream pricing can separate those partitions safely. + `model_source` distinguishes an independently identified provider model from + the exact requested model used when response identity is absent or ambiguous. + SkillSpector does not currently send Anthropic prompt-cache controls, so its + scan requests cannot select the separate 5-minute or 1-hour cache-write tiers; + TTL-specific response fields are normalized defensively into the aggregate + cache-write counter. +- See [Inference usage telemetry](docs/INFERENCE_USAGE.md) for the complete + provenance, cache-accounting, privacy, fail-closed ingestion, and downstream + pricing contract. - The full per-issue shape is defined by `Finding.to_dict()` in [models.py](src/skillspector/models.py); rely on the fields above and treat any additional fields as best-effort. For CI/IDE tooling, `--format sarif` emits SARIF 2.1.0. diff --git a/contrib/batch_scan/api_pool.py b/contrib/batch_scan/api_pool.py index d1ff0ea7..6960eab9 100644 --- a/contrib/batch_scan/api_pool.py +++ b/contrib/batch_scan/api_pool.py @@ -438,9 +438,22 @@ async def ainvoke(self, prompt: str) -> object: """Async invoke with automatic key switching on rate-limit.""" return await self._ainvoke_with_retry(prompt) + def invoke_with_usage(self, prompt: str, collector: object) -> object: + """Invoke while forwarding the telemetry callback to the selected model.""" + return self._invoke_with_retry(prompt, callbacks=[collector]) + + async def ainvoke_with_usage(self, prompt: str, collector: object) -> object: + """Async usage-aware counterpart to :meth:`invoke_with_usage`.""" + return await self._ainvoke_with_retry(prompt, callbacks=[collector]) + # -- Internal ------------------------------------------------------------- - def _invoke_with_retry(self, prompt: str) -> object: + def _invoke_with_retry( + self, + prompt: str, + *, + callbacks: list[object] | None = None, + ) -> object: """Sync retry loop — acquire slot, call LLM, release, retry on 429.""" last_exception: Exception | None = None @@ -448,7 +461,10 @@ def _invoke_with_retry(self, prompt: str) -> object: key = self._pool.acquire() llm = self._build_llm(key) try: - result = llm.invoke(prompt) + if callbacks is None: + result = llm.invoke(prompt) + else: + result = llm.invoke(prompt, config={"callbacks": callbacks}) self._pool.release(key, success=True) if attempt > 0: self._pool.record_retry_success() @@ -472,7 +488,12 @@ def _invoke_with_retry(self, prompt: str) -> object: "due to rate-limit errors" ) from last_exception - async def _ainvoke_with_retry(self, prompt: str) -> object: + async def _ainvoke_with_retry( + self, + prompt: str, + *, + callbacks: list[object] | None = None, + ) -> object: """Async retry loop — non-blocking acquire first, block only if full.""" import asyncio last_exception: Exception | None = None @@ -483,7 +504,10 @@ async def _ainvoke_with_retry(self, prompt: str) -> object: key = await asyncio.to_thread(self._pool.acquire) llm = self._build_llm(key) try: - result = await llm.ainvoke(prompt) + if callbacks is None: + result = await llm.ainvoke(prompt) + else: + result = await llm.ainvoke(prompt, config={"callbacks": callbacks}) self._pool.release(key, success=True) if attempt > 0: self._pool.record_retry_success() diff --git a/contrib/batch_scan/runner.py b/contrib/batch_scan/runner.py index ad008e60..1ac819ad 100644 --- a/contrib/batch_scan/runner.py +++ b/contrib/batch_scan/runner.py @@ -86,7 +86,9 @@ def set_api_pool(pool: "ApiKeyPool | None") -> None: def _pooled_get_chat_model(model=None): if _api_pool: from .api_pool import PooledChatModel - return PooledChatModel(_api_pool) + pooled_model = PooledChatModel(_api_pool) + _llm_utils.register_chat_model_provider(pooled_model, "openai") + return pooled_model return _original_get_chat_model(model) _llm_utils.get_chat_model = _pooled_get_chat_model @@ -120,7 +122,7 @@ def _pooled_get_chat_model(model=None): _original_base_init = LLMAnalyzerBase.__init__ -def _patched_base_init(self, base_prompt, model): +def _patched_base_init(self, base_prompt, model, *, node="llm_analyzer"): """Set response_schema=None on the instance dict BEFORE original init. Relies on Python MRO guarantee: instance.__dict__ is always checked @@ -128,7 +130,7 @@ def _patched_base_init(self, base_prompt, model): a library internal. """ self.response_schema = None - _original_base_init(self, base_prompt, model) + _original_base_init(self, base_prompt, model, node=node) # -- Patch 2: LLMAnalyzerBase.parse_response handles raw JSON -------------- @@ -316,13 +318,19 @@ def _verify_patch_targets() -> None: from skillspector.llm_analyzer_base import Batch, LLMFinding - # -- Patch 1: LLMAnalyzerBase.__init__(self, base_prompt, model) --------- + # -- Patch 1: LLMAnalyzerBase.__init__(..., *, node=...) ----------------- _check_signature( LLMAnalyzerBase.__init__, ["self", "base_prompt", "model"], "LLMAnalyzerBase.__init__", 1, ) + _node_param = inspect.signature(LLMAnalyzerBase.__init__).parameters.get("node") + if _node_param is None or _node_param.kind != inspect.Parameter.KEYWORD_ONLY: + raise RuntimeError( + "Patch 1 target changed: LLMAnalyzerBase.__init__ must retain its " + "keyword-only 'node' parameter." + ) if not hasattr(LLMAnalyzerBase, "response_schema"): raise RuntimeError( "Patch 1 target lost: LLMAnalyzerBase no longer has " diff --git a/contrib/batch_scan/tests/test_monkeypatch_fragility.py b/contrib/batch_scan/tests/test_monkeypatch_fragility.py index 26b55e8b..950cd077 100644 --- a/contrib/batch_scan/tests/test_monkeypatch_fragility.py +++ b/contrib/batch_scan/tests/test_monkeypatch_fragility.py @@ -39,6 +39,8 @@ import sys import unittest from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch _project_root = Path(__file__).resolve().parents[3] if str(_project_root) not in sys.path: @@ -60,6 +62,7 @@ _original_base_build_prompt, _original_meta_parse, _original_meta_build_prompt, + _patched_base_init, _verify_patch_targets, _apply_patches, _restore_patches, @@ -246,6 +249,37 @@ def _broken_init(self, base_prompt): finally: LLMAnalyzerBase.__init__ = original + def test_guard_catches_missing_node_param(self) -> None: + original = LLMAnalyzerBase.__init__ + + def _broken_init(self, base_prompt, model): + pass + + try: + LLMAnalyzerBase.__init__ = _broken_init + with self.assertRaisesRegex(RuntimeError, "node"): + _verify_patch_targets() + finally: + LLMAnalyzerBase.__init__ = original + + def test_patched_init_forwards_keyword_only_node(self) -> None: + instance = SimpleNamespace() + with patch("contrib.batch_scan.runner._original_base_init") as original_init: + _patched_base_init( + instance, + "prompt", + "model", + node="semantic_security_discovery", + ) + + original_init.assert_called_once_with( + instance, + "prompt", + "model", + node="semantic_security_discovery", + ) + self.assertIsNone(instance.response_schema) + def test_guard_catches_missing_response_schema_attr(self) -> None: """If upstream removes response_schema class attr, guard must raise.""" with _TempAttributeOverride(LLMAnalyzerBase, "response_schema", delete=True): diff --git a/contrib/batch_scan/tests/tests-pro/test_api_pool.py b/contrib/batch_scan/tests/tests-pro/test_api_pool.py index 208f42d4..1081f462 100644 --- a/contrib/batch_scan/tests/tests-pro/test_api_pool.py +++ b/contrib/batch_scan/tests/tests-pro/test_api_pool.py @@ -27,7 +27,7 @@ import time import unittest from pathlib import Path -from unittest.mock import patch +from unittest.mock import AsyncMock, MagicMock, patch _project_root = Path(__file__).resolve().parents[3] if str(_project_root) not in sys.path: @@ -39,6 +39,7 @@ PooledChatModel, create_api_key_pool_from_env, ) +from skillspector.llm_utils import _ainvoke_with_usage, _invoke_with_usage # --------------------------------------------------------------------------- @@ -459,5 +460,43 @@ def test_release_with_failure_does_not_leak_slot(self): self.assertEqual(pool.active_requests, 0) +class TestPooledUsageCallbacks(unittest.TestCase): + def test_sync_wrapper_forwards_collector_to_selected_langchain_model(self): + pool = _make_pool(n=1) + model = _make_pooled_model(pool) + collector = object() + response = object() + llm = MagicMock() + llm.invoke.return_value = response + + with patch.object(model, "_build_llm", return_value=llm): + result = _invoke_with_usage(model, "prompt", collector) + + self.assertIs(result, response) + llm.invoke.assert_called_once_with( + "prompt", + config={"callbacks": [collector]}, + ) + + +class TestPooledAsyncUsageCallbacks(unittest.IsolatedAsyncioTestCase): + async def test_async_wrapper_forwards_collector_to_selected_langchain_model(self): + pool = _make_pool(n=1) + model = _make_pooled_model(pool) + collector = object() + response = object() + llm = MagicMock() + llm.ainvoke = AsyncMock(return_value=response) + + with patch.object(model, "_build_llm", return_value=llm): + result = await _ainvoke_with_usage(model, "prompt", collector) + + self.assertIs(result, response) + llm.ainvoke.assert_awaited_once_with( + "prompt", + config={"callbacks": [collector]}, + ) + + if __name__ == "__main__": unittest.main() diff --git a/docs/INFERENCE_USAGE.md b/docs/INFERENCE_USAGE.md new file mode 100644 index 00000000..b8d128b5 --- /dev/null +++ b/docs/INFERENCE_USAGE.md @@ -0,0 +1,174 @@ +# Inference usage telemetry + +SkillSpector exposes provider-reported LLM usage in JSON reports so CI +consumers can calculate cost without scraping logs or estimating tokens. The +contract is intentionally raw: SkillSpector normalizes token counters and +model provenance, but it does not attach prices or calculate currency values. +This lets downstream systems apply an effective-dated pricing catalog without +rerunning a security scan. + +## JSON contract + +Run a scan with machine-readable output: + +```bash +skillspector scan ./my-skill --format json +``` + +Each successfully observed provider response contributes one entry to +`metadata.inference_usage`: + +```json +{ + "metadata": { + "llm_requested": true, + "llm_available": true, + "inference_usage": [ + { + "node": "semantic_security_discovery", + "request_kind": "structured_output", + "provider": "anthropic", + "model": "claude-opus-4-6", + "model_source": "provider_response", + "usage_source": "provider_response", + "prompt_tokens": 1000, + "completion_tokens": 100, + "cached_tokens": 400, + "cache_write_tokens": 50, + "reasoning_tokens": 25, + "total_tokens": 1100 + } + ] + } +} +``` + +| Field | Meaning | +|---|---| +| `node` | SkillSpector analyzer that made the request. | +| `request_kind` | Invocation shape, such as `structured_output` or `chat_completion`. | +| `provider` | Sanitized provider identifier; it never contains an endpoint or credential. | +| `model` | Provider-returned model identity when available, otherwise the exact requested model. | +| `model_source` | `provider_response` when the response unambiguously identified a different resolved model; `requested_model` when identity is absent or indistinguishable from a client-configured fallback. | +| `usage_source` | Always `provider_response`. SkillSpector does not emit estimated usage records. | +| `prompt_tokens` | Total normalized input tokens, inclusive of cache reads and cache writes. | +| `completion_tokens` | Provider-reported output tokens. | +| `cached_tokens` | Cache-read input tokens; a subset of `prompt_tokens`. | +| `cache_write_tokens` | Cache-creation input tokens; a subset of `prompt_tokens`. | +| `reasoning_tokens` | Provider-reported reasoning-token partition, normally a subset of completion usage. | +| `total_tokens` | Provider total, normalized to `prompt_tokens + completion_tokens` when both partitions are known. | + +Counter fields are optional because providers and transports expose different +levels of detail. A present zero is an observed zero. A missing field means the +provider did not expose that counter; it must not be treated as zero. + +## Model provenance + +`model_source` and `usage_source` answer different questions: + +- `usage_source=provider_response` means all token counters in the record came + from the completed provider response. SkillSpector never derives billing + counters from prompt length, local tokenizers, or analyzer token budgets. +- `model_source=provider_response` means the provider returned a valid model + identity distinguishable from the requested value. This is the strongest + identity for pricing because a gateway can route an alias to a different + deployed model. +- `model_source=requested_model` means the response had usage counters but no + independently verifiable model identity. This includes LangChain clients that + copy their configured model into response metadata when the provider omits + the field. `model` is then the exact model SkillSpector requested; downstream + pricing can use it, but should retain the weaker provenance. + +The configured model is resolved independently for each analyzer slot. The +general precedence is: + +1. `SKILLSPECTOR_MODEL_` +2. `SKILLSPECTOR_MODEL` +3. the active provider's default for that slot +4. the active provider's general default + +For example, `SKILLSPECTOR_MODEL_META_ANALYZER` affects only the +`meta_analyzer` slot, while `SKILLSPECTOR_MODEL` overrides every slot that has +no slot-specific override. A configured slot is not proof that a request ran. +Only a corresponding `inference_usage` record proves that SkillSpector received +a provider response with usage counters. + +## Cache and total-token semantics + +SkillSpector normalizes provider differences into one additive pricing shape: + +```text +uncached prompt = prompt_tokens - cached_tokens - cache_write_tokens +total tokens = prompt_tokens + completion_tokens +``` + +OpenAI-compatible responses generally report cache-read tokens as a partition +already included in prompt tokens. Raw Anthropic responses report ordinary +input, cache reads, and cache creation separately. SkillSpector adds the raw +Anthropic cache partitions exactly once so `prompt_tokens` is inclusive for +both response shapes. + +Anthropic cache-creation TTL details, when present, are combined into +`cache_write_tokens`. SkillSpector does not currently send prompt-cache +controls, so it does not choose between the separate 5-minute and 1-hour cache +write tiers. Downstream pricing must not infer a TTL that the provider response +did not preserve. + +`reasoning_tokens` is a diagnostic partition and must not be added to +`completion_tokens` a second time. Likewise, cache reads and cache writes must +not be added to `prompt_tokens` after normalization. + +## Missing usage and fail-closed integrations + +`metadata.inference_usage` is always a list in JSON output. An empty list means +usage was not observable. It does **not** mean that no LLM ran, that the request +was free, or that the token count was zero. Typical causes include a provider or +CLI transport that does not expose counters, an LLM call that failed before a +response, or a static-only scan. + +Cost observability and security-gate validity are separate decisions. A JSON +consumer should: + +1. require a parseable top-level JSON object; +2. treat a fatal process exit or `execution_successful: false` as a blocking + validation error; +3. surface `analysis_completeness.ledger_exceptions` for diagnosis; +4. apply its security policy to `risk_assessment.recommendation`; and +5. ingest every valid `inference_usage` record, including records preserved in + a failed LLM attempt, because a failed scan can still incur provider cost. + +Malformed telemetry must be discarded without turning an otherwise valid scan +into a failure. Conversely, valid usage telemetry must never make an incomplete +security scan pass. When an integrating tool retries a failed LLM scan in +static-only mode, it should ingest the failed attempt's usage once and avoid +double-counting the retry payload. + +## Privacy and trust boundary + +The report uses an explicit allowlist. Usage records contain only bounded +labels and non-negative provider counters. They do not contain prompts, +completions, analyzed skill content, credentials, headers, endpoint URLs, +provider request IDs, or raw provider metadata. Records with unknown sources, +invalid labels, negative or unbounded counters, or no counters are omitted. + +Treat the JSON report as untrusted input at every downstream boundary. Validate +the allowlisted fields and counter ranges again before appending metrics or +applying prices. + +## Downstream handoff + +The intended handoff is: + +```text +SkillSpector provider response + -> metadata.inference_usage in the SkillSpector JSON report + -> integrating evaluator validates and projects raw usage + -> CI publishes a versioned metrics artifact + -> dashboard applies an effective-dated pricing catalog +``` + +The evaluator should preserve `provider`, `model`, `model_source`, +`usage_source`, the analyzer/request identity, and every observed token +partition. Currency calculation belongs downstream so historical usage can be +repriced when a catalog is corrected without rewriting the original scan +artifact. diff --git a/docs/SUPPRESSION.md b/docs/SUPPRESSION.md index c99a0ec7..9a7065ca 100644 --- a/docs/SUPPRESSION.md +++ b/docs/SUPPRESSION.md @@ -39,6 +39,11 @@ skillspector scan ./my-skill/ --baseline .skillspector-baseline.yaml --show-supp | `skillspector scan --baseline FILE --show-suppressed` | Also list the suppressed findings (they still don't affect the score). | A missing, malformed, or unsupported baseline file exits with code 2. +When a selected baseline or baseline output is stored inside the scan target, +SkillSpector treats that exact file as an explicit scope exclusion. This +prevents sensitive rule text from creating a finding against itself or entering +regenerated fingerprints. Other baseline files and sibling YAML/JSON files +remain in normal scan scope unless they are selected with `--baseline` or `-o`. ## Baseline file format diff --git a/docs/release/skillspector-2.6.0.md b/docs/release/skillspector-2.6.0.md new file mode 100644 index 00000000..57215ed9 --- /dev/null +++ b/docs/release/skillspector-2.6.0.md @@ -0,0 +1,49 @@ +# SkillSpector v2.6.0 + +Released: 2026-08-05 + +## Summary + +This release includes 2 public-facing change(s) since release/2.5.3. + +## Highlights + +- feat(release): auto-generate versioned release notes like CHANGELOG +- feat(telemetry): export provider inference usage + +## Added + +- feat(release): auto-generate versioned release notes like CHANGELOG +- feat(telemetry): export provider inference usage + +## Changed + +- None. + +## Fixed + +- None. + +## Security + +- None. + +## Breaking Changes and Migration + +- None. + +## Deprecations + +- None. + +## Validation + +- Auto-generated from public-safe commit subjects since release/2.5.3; no additional validation commands were recorded by the release driver. + +## Known Limitations + +- None. + +## References + +- `CHANGELOG.md` diff --git a/docs/release/skillspector-2.7.0.md b/docs/release/skillspector-2.7.0.md new file mode 100644 index 00000000..a4ba0924 --- /dev/null +++ b/docs/release/skillspector-2.7.0.md @@ -0,0 +1,47 @@ +# SkillSpector v2.7.0 + +Released: 2026-08-06 + +## Summary + +This release includes 1 public-facing change(s) since release/2.6.0. + +## Highlights + +- fix(telemetry): harden inference usage normalization + +## Added + +- None. + +## Changed + +- None. + +## Fixed + +- fix(telemetry): harden inference usage normalization + +## Security + +- None. + +## Breaking Changes and Migration + +- None. + +## Deprecations + +- None. + +## Validation + +- Auto-generated from public-safe commit subjects since release/2.6.0; no additional validation commands were recorded by the release driver. + +## Known Limitations + +- None. + +## References + +- `CHANGELOG.md` diff --git a/docs/release/skillspector-2.7.2.md b/docs/release/skillspector-2.7.2.md new file mode 100644 index 00000000..6ca44df3 --- /dev/null +++ b/docs/release/skillspector-2.7.2.md @@ -0,0 +1,47 @@ +# SkillSpector v2.7.2 + +Released: 2026-08-06 + +## Summary + +This release includes 1 public-facing change(s) since release/2.7.0. + +## Highlights + +- fix(pe3): distinguish OAuth access-token nouns from credential access + +## Added + +- None. + +## Changed + +- None. + +## Fixed + +- fix(pe3): distinguish OAuth access-token nouns from credential access + +## Security + +- None. + +## Breaking Changes and Migration + +- None. + +## Deprecations + +- None. + +## Validation + +- Auto-generated from public-safe commit subjects since release/2.7.0; no additional validation commands were recorded by the release driver. + +## Known Limitations + +- None. + +## References + +- `CHANGELOG.md` diff --git a/docs/release/skillspector-2.8.0.md b/docs/release/skillspector-2.8.0.md new file mode 100644 index 00000000..e0220346 --- /dev/null +++ b/docs/release/skillspector-2.8.0.md @@ -0,0 +1,47 @@ +# SkillSpector v2.8.0 + +Released: 2026-08-06 + +## Summary + +This release includes 1 public-facing change(s) since release/2.7.2. + +## Highlights + +- fix(baseline): exclude selected baseline from scans + +## Added + +- None. + +## Changed + +- None. + +## Fixed + +- fix(baseline): exclude selected baseline from scans + +## Security + +- None. + +## Breaking Changes and Migration + +- None. + +## Deprecations + +- None. + +## Validation + +- Auto-generated from public-safe commit subjects since release/2.7.2; no additional validation commands were recorded by the release driver. + +## Known Limitations + +- None. + +## References + +- `CHANGELOG.md` diff --git a/docs/release/skillspector-2.8.1.md b/docs/release/skillspector-2.8.1.md new file mode 100644 index 00000000..ed0c0d95 --- /dev/null +++ b/docs/release/skillspector-2.8.1.md @@ -0,0 +1,47 @@ +# SkillSpector v2.8.1 + +Released: 2026-08-06 + +## Summary + +This release includes 1 public-facing change(s) since release/2.8.0. + +## Highlights + +- fix(llm): isolate malformed structured responses per batch + +## Added + +- None. + +## Changed + +- None. + +## Fixed + +- fix(llm): isolate malformed structured responses per batch + +## Security + +- None. + +## Breaking Changes and Migration + +- None. + +## Deprecations + +- None. + +## Validation + +- Auto-generated from public-safe commit subjects since release/2.8.0; no additional validation commands were recorded by the release driver. + +## Known Limitations + +- None. + +## References + +- `CHANGELOG.md` diff --git a/pyproject.toml b/pyproject.toml index ab464560..38d79171 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "skillspector" -version = "2.5.3" +version = "2.8.1" description = "SkillSpector: Security scanner for AI agent skills (Claude Code, Cursor, and similar). Scans skills for vulnerabilities, malicious patterns, and security risks before installation. Supports Git repos, URLs, zips, and local directories; runs static pattern checks and optional LLM semantic analysis; outputs terminal, JSON, and Markdown reports with risk scoring." readme = "README.md" license = "Apache-2.0" diff --git a/src/skillspector/cli.py b/src/skillspector/cli.py index 2519a7da..aa1ed658 100644 --- a/src/skillspector/cli.py +++ b/src/skillspector/cli.py @@ -137,6 +137,7 @@ def _scan_state( if baseline is not None: # Loading may raise FileNotFoundError/ValueError, mapped to exit code 2 by scan(). state["baseline"] = load_baseline(baseline) + state["baseline_path"] = os.path.abspath(baseline.expanduser()) state["show_suppressed"] = show_suppressed return state @@ -616,6 +617,7 @@ def baseline( console.print("[dim]Scanning to build baseline...[/dim]") # output_format is irrelevant here; we consume findings, not report_body. state = _scan_state(input_path, FormatChoice.json, no_llm) + state["baseline_path"] = os.path.abspath(output.expanduser()) result = graph.invoke(state) findings = result.get("filtered_findings") or result.get("findings") or [] data = build_baseline_dict( diff --git a/src/skillspector/inference_usage.py b/src/skillspector/inference_usage.py new file mode 100644 index 00000000..6726fd9b --- /dev/null +++ b/src/skillspector/inference_usage.py @@ -0,0 +1,397 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Sanitized provider-reported inference usage for scan reports. + +The collector is attached as a LangChain callback at invocation time. This is +important for structured output: the parser returns a Pydantic object and would +otherwise discard the provider message that carries token counters. +""" + +from __future__ import annotations + +import re +import threading +from collections.abc import Mapping, Sequence +from typing import NotRequired, TypedDict + +from langchain_core.callbacks import BaseCallbackHandler +from langchain_core.outputs import LLMResult + +_LABEL_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._:/@+\-]{0,255}") +_COUNTER_KEYS = ( + "prompt_tokens", + "completion_tokens", + "cached_tokens", + "cache_write_tokens", + "reasoning_tokens", + "total_tokens", +) +_MAX_TOKEN_COUNT = (1 << 63) - 1 + + +class InferenceUsageRecord(TypedDict): + """One provider-reported inference request, safe to serialize.""" + + node: str + request_kind: str + provider: str + model: str + model_source: str + usage_source: str + prompt_tokens: NotRequired[int] + completion_tokens: NotRequired[int] + cached_tokens: NotRequired[int] + cache_write_tokens: NotRequired[int] + reasoning_tokens: NotRequired[int] + total_tokens: NotRequired[int] + + +def _mapping(value: object) -> Mapping[str, object]: + return value if isinstance(value, Mapping) else {} + + +def _field(value: object, name: str) -> object | None: + if isinstance(value, Mapping): + return value.get(name) + return getattr(value, name, None) + + +def _counter(value: object) -> int | None: + if isinstance(value, bool): + return None + if isinstance(value, int) and 0 <= value <= _MAX_TOKEN_COUNT: + return value + if isinstance(value, float) and 0 <= value <= _MAX_TOKEN_COUNT and value.is_integer(): + return int(value) + return None + + +def _first_counter(*values: object) -> int | None: + for value in values: + parsed = _counter(value) + if parsed is not None: + return parsed + return None + + +def _positive_counter_sum(*values: object) -> int | None: + """Return a positive sum when provider-specific partitions are present.""" + counters = [parsed for value in values if (parsed := _counter(value)) is not None] + total = sum(counters) + return total if total > 0 else None + + +def _label(value: object, fallback: str = "unknown") -> str: + candidate = str(value or "").strip() + if _LABEL_RE.fullmatch(candidate): + return candidate + clean_fallback = str(fallback or "").strip() + return clean_fallback if _LABEL_RE.fullmatch(clean_fallback) else "unknown" + + +def _strict_label(value: object) -> str | None: + candidate = str(value or "").strip() + return candidate if _LABEL_RE.fullmatch(candidate) else None + + +def _strict_model_label(value: object) -> str | None: + """Return a model label only when it cannot encode a URL or userinfo.""" + candidate = _strict_label(value) + if candidate is None or "://" in candidate or "@" in candidate: + return None + return candidate + + +def _model_label(value: object, fallback: str = "unknown") -> str: + return _strict_model_label(value) or _strict_model_label(fallback) or "unknown" + + +def provider_name(provider: object) -> str: + """Return a stable provider label without endpoint or credential data.""" + names = { + "AnthropicProvider": "anthropic", + "AnthropicProxyProvider": "anthropic_proxy", + "BedrockProvider": "bedrock", + "ClaudeCLIProvider": "claude_cli", + "CodexCLIProvider": "codex_cli", + "GeminiCLIProvider": "gemini_cli", + "NvBuildProvider": "nv_build", + "NvInferenceProvider": "nv_inference", + "OpenAIProvider": "openai", + } + return names.get(type(provider).__name__, _label(type(provider).__name__.lower())) + + +def _usage_record( + message: object, + llm_output: Mapping[str, object], + *, + node: str, + request_kind: str, + provider: str, + requested_model: str, +) -> InferenceUsageRecord | None: + usage_metadata = _mapping(_field(message, "usage_metadata")) + response_metadata = _mapping(_field(message, "response_metadata")) + response_usage = _mapping(response_metadata.get("usage")) + token_usage = _mapping(response_metadata.get("token_usage")) + if not token_usage: + token_usage = _mapping(llm_output.get("token_usage")) + + input_details = _mapping( + usage_metadata.get("input_token_details") + or usage_metadata.get("input_tokens_details") + or token_usage.get("prompt_tokens_details") + or token_usage.get("input_tokens_details") + ) + output_details = _mapping( + usage_metadata.get("output_token_details") + or usage_metadata.get("output_tokens_details") + or token_usage.get("completion_tokens_details") + or token_usage.get("output_tokens_details") + ) + + standardized_prompt = _first_counter( + usage_metadata.get("input_tokens"), + usage_metadata.get("prompt_tokens"), + ) + # LangChain usage_metadata follows an inclusive input-token contract and + # carries cache partitions in input_token_details. Raw Anthropic usage is + # different: input_tokens excludes its separately reported cache fields. + # Use the raw-direct mode only when a standardized prompt total is absent. + # Some integrations populate unrelated usage metadata while leaving prompt + # accounting solely in the raw response. + direct_cache_read = ( + _first_counter( + response_usage.get("cache_read_input_tokens"), + token_usage.get("cache_read_input_tokens"), + ) + if standardized_prompt is None + else None + ) + raw_cache_creation = _mapping( + response_usage.get("cache_creation") or token_usage.get("cache_creation") + ) + raw_ttl_cache_write_tokens = _positive_counter_sum( + raw_cache_creation.get("ephemeral_5m_input_tokens"), + raw_cache_creation.get("ephemeral_1h_input_tokens"), + ) + direct_cache_write = ( + _first_counter( + raw_ttl_cache_write_tokens, + response_usage.get("cache_creation_input_tokens"), + token_usage.get("cache_creation_input_tokens"), + token_usage.get("cache_write_tokens"), + ) + if standardized_prompt is None + else None + ) + cached_tokens = _first_counter( + direct_cache_read, + input_details.get("cache_read"), + input_details.get("cached_tokens"), + usage_metadata.get("cache_read_input_tokens"), + response_usage.get("cache_read_input_tokens"), + token_usage.get("cache_read_input_tokens"), + ) + detail_ttl_cache_write_tokens = _positive_counter_sum( + input_details.get("ephemeral_5m_input_tokens"), + input_details.get("ephemeral_1h_input_tokens"), + ) + ttl_cache_write_tokens = detail_ttl_cache_write_tokens or raw_ttl_cache_write_tokens + cache_write_tokens = _first_counter( + ttl_cache_write_tokens, + direct_cache_write, + input_details.get("cache_creation"), + input_details.get("cache_write"), + input_details.get("cache_write_tokens"), + usage_metadata.get("cache_creation_input_tokens"), + response_usage.get("cache_creation_input_tokens"), + token_usage.get("cache_creation_input_tokens"), + token_usage.get("cache_write_tokens"), + ) + prompt_tokens = _first_counter( + standardized_prompt, + response_usage.get("input_tokens"), + response_usage.get("prompt_tokens"), + token_usage.get("prompt_tokens"), + token_usage.get("input_tokens"), + ) + completion_tokens = _first_counter( + usage_metadata.get("output_tokens"), + usage_metadata.get("completion_tokens"), + response_usage.get("output_tokens"), + response_usage.get("completion_tokens"), + token_usage.get("completion_tokens"), + token_usage.get("output_tokens"), + ) + + # Anthropic's raw response reports cache reads and writes outside + # ``input_tokens``. OpenAI-compatible nested cache counters are already a + # subset of prompt_tokens and therefore must not be added again. + if direct_cache_read is not None or direct_cache_write is not None: + prompt_tokens = (prompt_tokens or 0) + (direct_cache_read or 0) + (direct_cache_write or 0) + + reasoning_tokens = _first_counter( + output_details.get("reasoning"), + output_details.get("reasoning_tokens"), + usage_metadata.get("reasoning_tokens"), + token_usage.get("reasoning_tokens"), + ) + total_tokens = _first_counter( + usage_metadata.get("total_tokens"), + response_usage.get("total_tokens"), + token_usage.get("total_tokens"), + ) + if prompt_tokens is not None and completion_tokens is not None: + total_tokens = prompt_tokens + completion_tokens + + counters = { + "prompt_tokens": prompt_tokens, + "completion_tokens": completion_tokens, + "cached_tokens": cached_tokens, + "cache_write_tokens": cache_write_tokens, + "reasoning_tokens": reasoning_tokens, + "total_tokens": total_tokens, + } + if not any(value is not None for value in counters.values()): + return None + + provider_model = ( + response_metadata.get("model_name") + or response_metadata.get("model") + or response_metadata.get("model_id") + or llm_output.get("model_name") + or llm_output.get("model") + ) + requested_model_label = _model_label(requested_model) + provider_model_label = _strict_model_label(provider_model) + model = provider_model_label or requested_model_label + record: InferenceUsageRecord = { + "node": _label(node), + "request_kind": _label(request_kind), + "provider": _label(provider), + "model": model, + "model_source": ( + "provider_response" + if provider_model_label is not None and provider_model_label != requested_model_label + else "requested_model" + ), + "usage_source": "provider_response", + } + for key, value in counters.items(): + if value is not None: + record[key] = value # type: ignore[literal-required] + return record + + +class InferenceUsageCollector(BaseCallbackHandler): + """Collect one normalized record from each completed provider call.""" + + def __init__( + self, + *, + node: str, + request_kind: str, + provider: str, + requested_model: str, + ) -> None: + self._node = node + self._request_kind = request_kind + self._provider = provider + self._requested_model = requested_model + self._records: list[InferenceUsageRecord] = [] + self._response_received = False + self._lock = threading.Lock() + + def on_llm_end(self, response: LLMResult, **kwargs: object) -> None: + """Capture usage after a successful provider response.""" + message: object = None + for generation_group in response.generations: + for generation in generation_group: + candidate = getattr(generation, "message", None) + if candidate is not None: + message = candidate + break + if message is not None: + break + record = _usage_record( + message, + _mapping(response.llm_output), + node=self._node, + request_kind=self._request_kind, + provider=self._provider, + requested_model=self._requested_model, + ) + with self._lock: + self._response_received = True + if record is not None: + self._records.append(record) + + def mark_response_received(self) -> None: + """Record a completed response from a non-LangChain transport.""" + with self._lock: + self._response_received = True + + def set_provider(self, provider: str) -> None: + """Set the effective provider before the first response is observed.""" + label = _label(provider) + with self._lock: + if self._response_received and label != self._provider: + raise RuntimeError("cannot change inference provider after a response") + self._provider = label + + @property + def response_received(self) -> bool: + """Whether the provider returned, even when it reported no token usage.""" + with self._lock: + return self._response_received + + def snapshot(self) -> list[InferenceUsageRecord]: + """Return detached copies safe for graph-state serialization.""" + with self._lock: + return [record.copy() for record in self._records] + + +def sanitize_inference_usage( + records: Sequence[object] | None, +) -> list[InferenceUsageRecord]: + """Whitelist report fields and discard malformed or counter-less records.""" + sanitized: list[InferenceUsageRecord] = [] + for source in records or []: + if not isinstance(source, Mapping): + continue + if source.get("usage_source") != "provider_response": + continue + node = _strict_label(source.get("node")) + request_kind = _strict_label(source.get("request_kind")) + provider = _strict_label(source.get("provider")) + model = _strict_model_label(source.get("model")) + model_source = source.get("model_source") + if ( + node is None + or request_kind is None + or provider is None + or model is None + or not isinstance(model_source, str) + or model_source not in {"provider_response", "requested_model"} + ): + continue + record: InferenceUsageRecord = { + "node": node, + "request_kind": request_kind, + "provider": provider, + "model": model, + "model_source": model_source, + "usage_source": "provider_response", + } + found = False + for key in _COUNTER_KEYS: + value = _counter(source.get(key)) + if value is not None: + record[key] = value # type: ignore[literal-required] + found = True + if found: + sanitized.append(record) + return sanitized diff --git a/src/skillspector/inspection_ledger.py b/src/skillspector/inspection_ledger.py index 5bef0604..7beb3fe6 100644 --- a/src/skillspector/inspection_ledger.py +++ b/src/skillspector/inspection_ledger.py @@ -57,6 +57,7 @@ class LedgerReason(StrEnum): MANIFEST_ABSENT = "manifest_absent" NO_APPLICABLE_FILES = "no_applicable_files" OMS_SIGNATURE = "oms_signature" + BASELINE_FILE = "baseline_file" REASON_MESSAGES: Final[dict[LedgerReason, str]] = { @@ -89,6 +90,9 @@ class LedgerReason(StrEnum): LedgerReason.OMS_SIGNATURE: ( "Recognized OMS signature metadata is excluded from content analysis." ), + LedgerReason.BASELINE_FILE: ( + "The explicitly selected suppression baseline is excluded from content analysis." + ), } diff --git a/src/skillspector/llm_analyzer_base.py b/src/skillspector/llm_analyzer_base.py index 4ad6c558..9aff5ed9 100644 --- a/src/skillspector/llm_analyzer_base.py +++ b/src/skillspector/llm_analyzer_base.py @@ -34,7 +34,7 @@ from typing import Any, Literal, cast from langchain_core.messages import BaseMessage -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, Field, ValidationError, field_validator from skillspector.inspection_ledger import ( AnalyzerStatusEvent, @@ -44,7 +44,13 @@ analyzer_status_event, ledger_event, ) -from skillspector.llm_utils import get_chat_model +from skillspector.llm_utils import ( + _AgentCLIMessage, + _ainvoke_with_usage, + _invoke_with_usage, + get_chat_model, + new_inference_usage_collector, +) from skillspector.logging_config import get_logger from skillspector.model_info import get_max_input_tokens from skillspector.models import Finding @@ -52,6 +58,11 @@ logger = get_logger(__name__) DEFAULT_MAX_LLM_CONCURRENCY = 10 +STRUCTURED_RESPONSE_MAX_ATTEMPTS = 2 + + +class _StructuredResponseValidationError(Exception): + """Signal that provider output failed structured-response validation.""" def resolve_max_concurrency() -> int: @@ -389,6 +400,13 @@ def _message_text(response: object) -> str: return str(response.text) +def _raw_response_text(response: object) -> str: + """Extract raw analyzer text from LangChain and CLI adapter messages.""" + if isinstance(response, _AgentCLIMessage): + return str(response.content) + return _message_text(response) + + BASE_ANALYSIS_PROMPT = """\ {analyzer_prompt} @@ -434,7 +452,7 @@ class LLMAnalyzerBase: response_schema: type | None = LLMAnalysisResult - def __init__(self, base_prompt: str, model: str): + def __init__(self, base_prompt: str, model: str, *, node: str = "llm_analyzer"): self.base_prompt = base_prompt self.model = model self._input_budget = get_max_input_tokens(model) @@ -442,6 +460,22 @@ def __init__(self, base_prompt: str, model: str): self._structured_llm = ( self._llm.with_structured_output(self.response_schema) if self.response_schema else None ) + self._usage_collector = new_inference_usage_collector( + node=node, + request_kind="structured_output" if self.response_schema else "chat_completion", + model=model, + chat_model=self._llm, + ) + + @property + def inference_usage(self) -> list[dict[str, object]]: + """Provider-reported usage captured for this analyzer instance.""" + return list(self._usage_collector.snapshot()) + + @property + def response_received(self) -> bool: + """Whether any analyzer call received a provider response.""" + return self._usage_collector.response_received # -- Batching ----------------------------------------------------------- @@ -530,6 +564,48 @@ def parse_response(self, response: object, batch: Batch) -> list[Finding]: # -- Run loop ----------------------------------------------------------- + def _invoke_batch(self, batch: Batch, prompt: str) -> tuple[Batch, list]: + """Invoke and parse one batch synchronously.""" + logger.debug( + "LLM call for %s (tokens~%d, findings=%d)", + batch.file_label, + estimate_tokens(prompt), + len(batch.findings), + ) + if self._structured_llm: + try: + response = _invoke_with_usage(self._structured_llm, prompt, self._usage_collector) + except ValidationError as exc: + raise _StructuredResponseValidationError from exc + else: + response = _raw_response_text( + _invoke_with_usage(self._llm, prompt, self._usage_collector) + ) + logger.debug("LLM response for %s", batch.file_label) + return batch, self.parse_response(response, batch) + + async def _ainvoke_batch(self, batch: Batch, prompt: str) -> tuple[Batch, list]: + """Invoke and parse one batch asynchronously.""" + logger.debug( + "LLM call for %s (tokens~%d, findings=%d)", + batch.file_label, + estimate_tokens(prompt), + len(batch.findings), + ) + if self._structured_llm: + try: + response = await _ainvoke_with_usage( + self._structured_llm, prompt, self._usage_collector + ) + except ValidationError as exc: + raise _StructuredResponseValidationError from exc + else: + response = _raw_response_text( + await _ainvoke_with_usage(self._llm, prompt, self._usage_collector) + ) + logger.debug("LLM response for %s", batch.file_label) + return batch, self.parse_response(response, batch) + def run_batches( self, batches: list[Batch], @@ -555,18 +631,24 @@ def run_batches_detailed( for batch in batches: try: prompt = self.build_prompt(batch, **kwargs) - logger.debug( - "LLM call for %s (tokens~%d, findings=%d)", + try: + result = self._invoke_batch(batch, prompt) + except _StructuredResponseValidationError: + logger.warning( + "LLM structured response validation failed for %s; retrying once", + batch.file_label, + ) + result = self._invoke_batch(batch, prompt) + outcome.successful.append(result) + except _StructuredResponseValidationError: + logger.warning( + "LLM structured response validation failed for %s after %d attempts", batch.file_label, - estimate_tokens(prompt), - len(batch.findings), + STRUCTURED_RESPONSE_MAX_ATTEMPTS, + ) + outcome.failures.append( + BatchFailure(batch=batch, error_class=ValidationError.__name__) ) - if self._structured_llm: - response = self._structured_llm.invoke(prompt) - else: - response = _message_text(self._llm.invoke(prompt)) - logger.debug("LLM response for %s", batch.file_label) - outcome.successful.append((batch, self.parse_response(response, batch))) except (ValueError, NotImplementedError): raise except Exception as exc: @@ -595,10 +677,12 @@ async def arun_batches( Failures are isolated per batch: a transient error (timeout, 429, oversized-chunk 400, ...) costs only its own batch, which is logged and omitted from the result, so one bad call cannot cancel the rest - of the fan-out. Callers can detect partial results by comparing the - returned batches against the submitted ones. ``ValueError`` and - ``NotImplementedError`` signal misconfiguration rather than infra - trouble and keep propagating. + of the fan-out. Malformed structured responses (Pydantic + ``ValidationError``) are retried once and then isolated to their batch. + Callers can detect partial results by comparing the returned batches + against the submitted ones. Other ``ValueError`` instances and + ``NotImplementedError`` signal misconfiguration rather than infra trouble + and keep propagating. The return type mirrors :meth:`run_batches`. """ @@ -623,22 +707,28 @@ async def arun_batches_detailed( async def _process(batch: Batch) -> tuple[Batch, list]: async with sem: prompt = self.build_prompt(batch, **kwargs) - logger.debug( - "LLM call for %s (tokens~%d, findings=%d)", - batch.file_label, - estimate_tokens(prompt), - len(batch.findings), - ) - if self._structured_llm: - response = await self._structured_llm.ainvoke(prompt) - else: - response = _message_text(await self._llm.ainvoke(prompt)) - logger.debug("LLM response for %s", batch.file_label) - return (batch, self.parse_response(response, batch)) + try: + return await self._ainvoke_batch(batch, prompt) + except _StructuredResponseValidationError: + logger.warning( + "LLM structured response validation failed for %s; retrying once", + batch.file_label, + ) + return await self._ainvoke_batch(batch, prompt) results = await asyncio.gather(*[_process(b) for b in batches], return_exceptions=True) outcome = BatchExecutionResult() for batch, result in zip(batches, results, strict=True): + if isinstance(result, _StructuredResponseValidationError): + logger.warning( + "LLM structured response validation failed for %s after %d attempts", + batch.file_label, + STRUCTURED_RESPONSE_MAX_ATTEMPTS, + ) + outcome.failures.append( + BatchFailure(batch=batch, error_class=ValidationError.__name__) + ) + continue if isinstance(result, (ValueError, NotImplementedError)): raise result if isinstance(result, BaseException): diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index faac3761..8b5ca4bb 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -37,14 +37,19 @@ import asyncio import concurrent.futures import json +import threading +import weakref from collections.abc import Coroutine from typing import Any, NoReturn from langchain_core.language_models.chat_models import BaseChatModel +from langchain_core.runnables import Runnable +from skillspector.inference_usage import InferenceUsageCollector, provider_name from skillspector.model_info import get_max_input_tokens, get_max_output_tokens from skillspector.providers import ( create_chat_model, + create_chat_model_with_provider, get_active_provider, get_metadata_provider, has_cli_capability, @@ -55,6 +60,37 @@ ) from skillspector.providers.openai import OpenAIProvider +_CHAT_MODEL_PROVIDERS: dict[int, tuple[weakref.ReferenceType[object], str]] = {} +_CHAT_MODEL_PROVIDERS_LOCK = threading.Lock() + + +def register_chat_model_provider(chat_model: object, provider: object) -> None: + """Associate a constructed chat model with its effective provider.""" + model_id = id(chat_model) + label = provider if isinstance(provider, str) else provider_name(provider) + + def _discard(model_ref: weakref.ReferenceType[object]) -> None: + with _CHAT_MODEL_PROVIDERS_LOCK: + current = _CHAT_MODEL_PROVIDERS.get(model_id) + if current is not None and current[0] is model_ref: + _CHAT_MODEL_PROVIDERS.pop(model_id, None) + + try: + model_ref = weakref.ref(chat_model, _discard) + except TypeError: + return + with _CHAT_MODEL_PROVIDERS_LOCK: + _CHAT_MODEL_PROVIDERS[model_id] = (model_ref, str(label)) + + +def chat_model_provider_name(chat_model: object) -> str | None: + """Return the provider recorded by the model-construction dispatch.""" + with _CHAT_MODEL_PROVIDERS_LOCK: + current = _CHAT_MODEL_PROVIDERS.get(id(chat_model)) + if current is None or current[0]() is not chat_model: + return None + return current[1] + def _resolve_llm_credentials() -> tuple[str, str | None]: """Return ``(api_key, base_url)`` resolved from the environment. @@ -195,17 +231,39 @@ def _augment(self, prompt: str) -> str: f"before or after the JSON.\n\nJSON Schema:\n{schema_json}" ) - def invoke(self, prompt: str) -> object: - raw = self._provider.complete( # type: ignore[attr-defined] + def _complete(self, prompt: str) -> str: + """Return provider output before structured parsing begins.""" + return self._provider.complete( # type: ignore[attr-defined,no-any-return] self._augment(prompt), model=self._model, max_output_tokens=self._max_output_tokens, ) + + def invoke(self, prompt: str) -> object: + raw = self._complete(prompt) return self._schema.model_validate(_extract_json_object(raw)) async def ainvoke(self, prompt: str) -> object: return await asyncio.to_thread(self.invoke, prompt) + def invoke_with_usage( + self, + prompt: str, + collector: InferenceUsageCollector, + ) -> object: + """Mark this invocation after transport success and before parsing.""" + raw = self._complete(prompt) + collector.mark_response_received() + return self._schema.model_validate(_extract_json_object(raw)) + + async def ainvoke_with_usage( + self, + prompt: str, + collector: InferenceUsageCollector, + ) -> object: + """Async counterpart to :meth:`invoke_with_usage`.""" + return await asyncio.to_thread(self.invoke_with_usage, prompt, collector) + class AgentCLIChatModel: """Minimal ``ChatOpenAI``-compatible adapter backed by a CLI provider. @@ -271,17 +329,81 @@ def get_chat_model(model: str | None = None) -> BaseChatModel | AgentCLIChatMode provider = get_active_provider() if has_cli_capability(provider): resolved_model = model or provider.resolve_model() - return AgentCLIChatModel(provider, resolved_model, get_max_output_tokens(resolved_model)) + chat_model = AgentCLIChatModel( + provider, + resolved_model, + get_max_output_tokens(resolved_model), + ) + register_chat_model_provider(chat_model, provider) + return chat_model model = model or _resolve_default_chat_model() - return create_chat_model( + chat_model, effective_provider = create_chat_model_with_provider( model=model, max_tokens=get_max_output_tokens(model), timeout=120, ) + register_chat_model_provider(chat_model, effective_provider) + return chat_model + + +def _invoke_with_usage(runnable: object, prompt: str, collector: InferenceUsageCollector) -> object: + """Invoke a LangChain runnable with telemetry without changing CLI adapters.""" + if isinstance(runnable, Runnable): + return runnable.invoke(prompt, config={"callbacks": [collector]}) + if isinstance(runnable, _StructuredAgentCLIModel): + return runnable.invoke_with_usage(prompt, collector) + invoke_with_usage = getattr(type(runnable), "invoke_with_usage", None) + if callable(invoke_with_usage): + return invoke_with_usage(runnable, prompt, collector) + if isinstance(runnable, AgentCLIChatModel): + response = runnable.invoke(prompt) + collector.mark_response_received() + return response + return runnable.invoke(prompt) # type: ignore[attr-defined] + + +async def _ainvoke_with_usage( + runnable: object, prompt: str, collector: InferenceUsageCollector +) -> object: + """Async counterpart to :func:`_invoke_with_usage`.""" + if isinstance(runnable, Runnable): + return await runnable.ainvoke(prompt, config={"callbacks": [collector]}) + if isinstance(runnable, _StructuredAgentCLIModel): + return await runnable.ainvoke_with_usage(prompt, collector) + ainvoke_with_usage = getattr(type(runnable), "ainvoke_with_usage", None) + if callable(ainvoke_with_usage): + return await ainvoke_with_usage(runnable, prompt, collector) + if isinstance(runnable, AgentCLIChatModel): + response = await runnable.ainvoke(prompt) + collector.mark_response_received() + return response + return await runnable.ainvoke(prompt) # type: ignore[attr-defined] + + +def new_inference_usage_collector( + *, node: str, request_kind: str, model: str, chat_model: object | None = None +) -> InferenceUsageCollector: + """Build a collector labeled with the provider that will handle the call.""" + effective_provider = ( + chat_model_provider_name(chat_model) if chat_model is not None else None + ) or provider_name(get_active_provider()) + return InferenceUsageCollector( + node=node, + request_kind=request_kind, + provider=effective_provider, + requested_model=model, + ) -def chat_completion(prompt: str, *, model: str | None = None) -> str: +def chat_completion( + prompt: str, + *, + model: str | None = None, + usage_collector: InferenceUsageCollector | None = None, + node: str = "chat_completion", + request_kind: str = "chat_completion", +) -> str: """Request a single chat completion and return the assistant content. Routes through :func:`get_chat_model`, which dispatches to the CLI adapter @@ -291,7 +413,24 @@ def chat_completion(prompt: str, *, model: str | None = None) -> str: which normalise content blocks to a single string) and falls back to ``.content`` for the CLI adapter's ``_AgentCLIMessage``. """ - response = get_chat_model(model=model).invoke(prompt) + chat_model = get_chat_model(model=model) + active_provider = get_active_provider() + resolved_model = str( + model + or getattr(chat_model, "model_name", None) + or getattr(chat_model, "model", None) + or active_provider.resolve_model() + ) + collector = usage_collector or new_inference_usage_collector( + node=node, + request_kind=request_kind, + model=resolved_model, + chat_model=chat_model, + ) + effective_provider = chat_model_provider_name(chat_model) + if usage_collector is not None and effective_provider is not None: + collector.set_provider(effective_provider) + response = _invoke_with_usage(chat_model, prompt, collector) if hasattr(response, "text"): return response.text # type: ignore[union-attr] return response.content or "" # type: ignore[union-attr] diff --git a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py index f6c70877..9898854a 100644 --- a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py +++ b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py @@ -23,14 +23,16 @@ import re import unicodedata +from skillspector.inference_usage import InferenceUsageCollector, InferenceUsageRecord from skillspector.inspection_ledger import ( LedgerOutcome, LedgerReason, analyzer_status_event, ledger_event, ) -from skillspector.llm_utils import chat_completion +from skillspector.llm_utils import chat_completion, new_inference_usage_collector from skillspector.models import Finding +from skillspector.providers import get_active_provider from skillspector.state import ( AnalyzerNodeResponse, LLMCallRecord, @@ -688,20 +690,29 @@ def _check_tp3(params: list[dict]) -> list[Finding]: ) -def _check_tp4(state: SkillspectorState) -> tuple[list[Finding], LLMCallRecord | None, str | None]: +def _check_tp4( + state: SkillspectorState, +) -> tuple[ + list[Finding], + LLMCallRecord | None, + str | None, + list[InferenceUsageRecord], +]: """TP4: LLM-based description-behavior mismatch detection. - Returns ``(findings, record, error_class)`` where *record* is the LLM-call telemetry for - ``llm_call_log`` — or ``None`` when no LLM call was attempted (no - description / no executable code), so an intentional no-op is never counted - as a degraded LLM stage. See :func:`skillspector.state.llm_call_record`. + Returns ``(findings, record, error_class, inference_usage)`` where + *record* is the LLM-call telemetry for ``llm_call_log`` — or ``None`` when + no LLM call was attempted (no description / no executable code), so an + intentional no-op is never counted as a degraded LLM stage. Token usage is + emitted only when the provider response supplied it. """ attempted = False + usage_collector: InferenceUsageCollector | None = None try: manifest: dict = state.get("manifest") or {} description = manifest.get("description") if not description or not isinstance(description, str) or not description.strip(): - return [], None, None + return [], None, None, [] triggers = manifest.get("triggers") or [] permissions = manifest.get("permissions") @@ -723,12 +734,17 @@ def _check_tp4(state: SkillspectorState) -> tuple[list[Finding], LLMCallRecord | code_parts.append(f"### {path} ({file_type})\n{content}") if not code_parts: - return [], None, None + return [], None, None, [] code_contents = "\n\n".join(code_parts) model_config: dict = state.get("model_config") or {} model = model_config.get(ANALYZER_ID) or model_config.get("default") + usage_collector = new_inference_usage_collector( + node=ANALYZER_ID, + request_kind="chat_completion", + model=model or get_active_provider().resolve_model(), + ) prompt = f"""You are a security auditor. Your task: determine whether a skill's declared description accurately represents what its code actually does. @@ -768,7 +784,12 @@ def _check_tp4(state: SkillspectorState) -> tuple[list[Finding], LLMCallRecord | }}""" attempted = True - response = chat_completion(prompt, model=model) + response = chat_completion( + prompt, + model=model, + usage_collector=usage_collector, + node=ANALYZER_ID, + ) # Parse JSON — handle optional ```json code blocks json_text = response.strip() @@ -785,11 +806,11 @@ def _check_tp4(state: SkillspectorState) -> tuple[list[Finding], LLMCallRecord | ok_record = llm_call_record(ANALYZER_ID, ok=True) if not result.get("is_mismatch"): - return [], ok_record, None + return [], ok_record, None, usage_collector.snapshot() confidence = float(result.get("confidence", 0.0)) if confidence < 0.5: - return [], ok_record, None + return [], ok_record, None, usage_collector.snapshot() severity = "HIGH" if confidence >= 0.7 else "MEDIUM" @@ -821,6 +842,7 @@ def _check_tp4(state: SkillspectorState) -> tuple[list[Finding], LLMCallRecord | ], ok_record, None, + usage_collector.snapshot(), ) except Exception as exc: @@ -828,8 +850,13 @@ def _check_tp4(state: SkillspectorState) -> tuple[list[Finding], LLMCallRecord | # Only record a failure if the LLM call was actually attempted; a failure # before the call (e.g. building the prompt) is not an LLM-stage failure. if attempted: - return [], llm_call_record(ANALYZER_ID, ok=False, error=str(exc)), type(exc).__name__ - return [], None, None + return ( + [], + llm_call_record(ANALYZER_ID, ok=False, error=str(exc)), + type(exc).__name__, + usage_collector.snapshot() if usage_collector is not None else [], + ) + return [], None, None, [] # --------------------------------------------------------------------------- @@ -891,8 +918,9 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: tp4_record: LLMCallRecord | None = None tp4_findings: list[Finding] = [] tp4_error_class: str | None = None + tp4_usage: list[InferenceUsageRecord] = [] if state.get("use_llm", True): - tp4_findings, tp4_record, tp4_error_class = _check_tp4(state) + tp4_findings, tp4_record, tp4_error_class, tp4_usage = _check_tp4(state) findings.extend(tp4_findings) logger.info("%s: %d findings", ANALYZER_ID, len(findings)) @@ -929,4 +957,5 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: # degradation detector counts this node consistently with the semantic ones. if tp4_record is not None: result["llm_call_log"] = [tp4_record] + result["inference_usage"] = tp4_usage return result diff --git a/src/skillspector/nodes/analyzers/semantic_developer_intent.py b/src/skillspector/nodes/analyzers/semantic_developer_intent.py index 9f35d1ff..e67e03e4 100644 --- a/src/skillspector/nodes/analyzers/semantic_developer_intent.py +++ b/src/skillspector/nodes/analyzers/semantic_developer_intent.py @@ -26,6 +26,7 @@ from skillspector.inspection_ledger import LedgerReason, analyzer_status_event from skillspector.llm_analyzer_base import ( BatchExecutionResult, + BatchFailure, LLMAnalyzerBase, ledger_events_for_batches, ) @@ -196,9 +197,11 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: or _SKILLSPECTOR_DEFAULT_MODEL ) + analyzer: LLMAnalyzerBase | None = None + batches = [] try: prompt = ANALYZER_PROMPT.format(manifest_section=_format_manifest(manifest)) - analyzer = LLMAnalyzerBase(base_prompt=prompt, model=model) + analyzer = LLMAnalyzerBase(base_prompt=prompt, model=model, node=ANALYZER_ID) batches = analyzer.get_batches(sorted(file_cache), file_cache) results = run_async(analyzer.arun_batches(batches)) outcome = getattr(analyzer, "_last_batch_outcome", BatchExecutionResult(successful=results)) @@ -212,19 +215,32 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: "llm_call_log": [ llm_call_record(ANALYZER_ID, ok=bool(outcome.successful) or not outcome.failures) ], + "inference_usage": analyzer.inference_usage, } - except ValueError: - raise except Exception as exc: + post_response_value_error = ( + isinstance(exc, ValueError) and analyzer is not None and analyzer.response_received + ) + if isinstance(exc, ValueError) and not post_response_value_error: + raise logger.warning("%s failed: %s", ANALYZER_ID, exc) + if post_response_value_error: + events, status = ledger_events_for_batches( + ANALYZER_ID, + BatchExecutionResult( + failures=[ + BatchFailure(batch=batch, error_class=type(exc).__name__) + for batch in batches + ] + ), + ) + else: + events = [] + status = analyzer_status_event(analyzer_id=ANALYZER_ID, status="unavailable") return { "findings": [], - "inspection_ledger": [], - "analyzer_status_events": [ - analyzer_status_event( - analyzer_id=ANALYZER_ID, - status="unavailable", - ) - ], + "inspection_ledger": events, + "analyzer_status_events": [status], "llm_call_log": [llm_call_record(ANALYZER_ID, ok=False, error=str(exc))], + "inference_usage": analyzer.inference_usage if analyzer is not None else [], } diff --git a/src/skillspector/nodes/analyzers/semantic_quality_policy.py b/src/skillspector/nodes/analyzers/semantic_quality_policy.py index d38c4955..2778da52 100644 --- a/src/skillspector/nodes/analyzers/semantic_quality_policy.py +++ b/src/skillspector/nodes/analyzers/semantic_quality_policy.py @@ -26,6 +26,7 @@ from skillspector.inspection_ledger import LedgerReason, analyzer_status_event from skillspector.llm_analyzer_base import ( BatchExecutionResult, + BatchFailure, LLMAnalyzerBase, ledger_events_for_batches, ) @@ -166,8 +167,10 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: model_config.get(ANALYZER_ID) or model_config.get("default") or _SKILLSPECTOR_DEFAULT_MODEL ) + analyzer: LLMAnalyzerBase | None = None + batches = [] try: - analyzer = LLMAnalyzerBase(base_prompt=ANALYZER_PROMPT, model=model) + analyzer = LLMAnalyzerBase(base_prompt=ANALYZER_PROMPT, model=model, node=ANALYZER_ID) batches = analyzer.get_batches(files, file_cache) results = run_async(analyzer.arun_batches(batches)) outcome = getattr(analyzer, "_last_batch_outcome", BatchExecutionResult(successful=results)) @@ -181,19 +184,32 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: "llm_call_log": [ llm_call_record(ANALYZER_ID, ok=bool(outcome.successful) or not outcome.failures) ], + "inference_usage": analyzer.inference_usage, } - except ValueError: - raise except Exception as exc: + post_response_value_error = ( + isinstance(exc, ValueError) and analyzer is not None and analyzer.response_received + ) + if isinstance(exc, ValueError) and not post_response_value_error: + raise logger.warning("%s failed: %s", ANALYZER_ID, exc) + if post_response_value_error: + events, status = ledger_events_for_batches( + ANALYZER_ID, + BatchExecutionResult( + failures=[ + BatchFailure(batch=batch, error_class=type(exc).__name__) + for batch in batches + ] + ), + ) + else: + events = [] + status = analyzer_status_event(analyzer_id=ANALYZER_ID, status="unavailable") return { "findings": [], - "inspection_ledger": [], - "analyzer_status_events": [ - analyzer_status_event( - analyzer_id=ANALYZER_ID, - status="unavailable", - ) - ], + "inspection_ledger": events, + "analyzer_status_events": [status], "llm_call_log": [llm_call_record(ANALYZER_ID, ok=False, error=str(exc))], + "inference_usage": analyzer.inference_usage if analyzer is not None else [], } diff --git a/src/skillspector/nodes/analyzers/semantic_security_discovery.py b/src/skillspector/nodes/analyzers/semantic_security_discovery.py index 7c70dd81..09bf2b2a 100644 --- a/src/skillspector/nodes/analyzers/semantic_security_discovery.py +++ b/src/skillspector/nodes/analyzers/semantic_security_discovery.py @@ -150,8 +150,9 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: ) batches: list[Batch] = [] + analyzer: LLMAnalyzerBase | None = None try: - analyzer = LLMAnalyzerBase(base_prompt=ANALYZER_PROMPT, model=model) + analyzer = LLMAnalyzerBase(base_prompt=ANALYZER_PROMPT, model=model, node=ANALYZER_ID) batches = analyzer.get_batches(available_components, file_cache) results = analyzer.run_batches(batches) outcome = getattr(analyzer, "_last_batch_outcome", BatchExecutionResult(successful=results)) @@ -180,6 +181,7 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: "llm_call_log": [ llm_call_record(ANALYZER_ID, ok=bool(outcome.successful) or not outcome.failures) ], + "inference_usage": analyzer.inference_usage, } except ValidationError as exc: # Malformed LLM response — degrade gracefully rather than crashing the graph @@ -211,19 +213,43 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: "llm_call_log": [ llm_call_record(ANALYZER_ID, ok=False, error=f"malformed LLM response: {exc}") ], + "inference_usage": analyzer.inference_usage if analyzer is not None else [], } - except ValueError: - raise except Exception as exc: + post_response_value_error = ( + isinstance(exc, ValueError) and analyzer is not None and analyzer.response_received + ) + if isinstance(exc, ValueError) and not post_response_value_error: + raise logger.warning("%s failed: %s", ANALYZER_ID, exc) + if post_response_value_error: + outcome = BatchExecutionResult( + failures=[ + BatchFailure(batch=batch, error_class=type(exc).__name__) for batch in batches + ] + ) + events, _ = ledger_events_for_batches(ANALYZER_ID, outcome) + all_events = [*missing_cache_events, *events] + status = analyzer_status_event( + analyzer_id=ANALYZER_ID, + status="failed", + planned_work=[ + { + "work_id": event["work_id"], + "path": event["path"], + "start_line": event["start_line"], + "end_line": event["end_line"], + } + for event in all_events + ], + ) + else: + all_events = list(missing_cache_events) + status = analyzer_status_event(analyzer_id=ANALYZER_ID, status="unavailable") return { "findings": [], - "inspection_ledger": [], - "analyzer_status_events": [ - analyzer_status_event( - analyzer_id=ANALYZER_ID, - status="unavailable", - ) - ], + "inspection_ledger": all_events, + "analyzer_status_events": [status], "llm_call_log": [llm_call_record(ANALYZER_ID, ok=False, error=str(exc))], + "inference_usage": analyzer.inference_usage if analyzer is not None else [], } diff --git a/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py b/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py index 6cecad33..c89760bf 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py +++ b/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py @@ -159,6 +159,40 @@ def _is_read_only_passwd_volume_match(content: str, match: re.Match[str]) -> boo r"(?Paccess\s+tokens?)\s*[`.)]*\s*$", re.IGNORECASE, ) +_PE3_TOKEN_LIFECYCLE_CONTEXT = re.compile( + r"\b(?:expires?|expiry|lifespan|refresh[ _-]?token|oauth|bearer|token endpoint|" + r"renew(?:ed|al|ing)?|revoked)\b" + r"|grant_type\s*=\s*client_credentials|(?:^|\W)401(?:\W|$)", + re.IGNORECASE, +) +_PE3_TOKEN_ACTION_CONTEXT = re.compile( + r"\b(?:steal|exfiltrat\w*|dump|harvest|scrape|upload|transmit|send|copy|extract)\b" + r"|\b(?:read|open|load)\b[^\n]{0,80}\b(?:access|refresh|bearer|api)[ _-]?tokens?\b", + re.IGNORECASE, +) +_PE3_TOKEN_SENSITIVE_SOURCE = re.compile( + r"(?#`]+\s*)*(?:\*{0,2}lifespan\s*:\s*\*{0,2}\s*)?", + re.IGNORECASE, +) +_PE3_ACCESS_TOKEN_LIFESPAN_SUFFIX = re.compile( + r"\s*(?:~?\d|expires?|is\s+(?:valid|used)|lasts?\b)", + re.IGNORECASE, +) +_PE3_TOKEN_LIFECYCLE_DOCUMENTATION_DIRS = frozenset( + {"docs", "documentation", "procedures", "references", "examples", "guides"} +) def _source_line(content: str, match: re.Match[str]) -> str: @@ -170,6 +204,48 @@ def _source_line(content: str, match: re.Match[str]) -> str: return content[line_start:line_end] +def _is_access_token_lifecycle_noun( + content: str, + match: re.Match[str], + file_type: str, + file_path: str, +) -> bool: + """Return True for a bounded OAuth ``access token`` noun in documentation. + + PE3's generic ``access … tokens?`` rule cannot distinguish the verb + "access tokens" from the OAuth compound noun "access token". Suppress only + noun-shaped matches with nearby lifecycle evidence, and fail closed when + the context contains credential actions or sensitive sources. + """ + if file_type not in {"markdown", "text"}: + return False + normalized_parts = file_path.replace("\\", "/").lower().split("/") + if not any(part in _PE3_TOKEN_LIFECYCLE_DOCUMENTATION_DIRS for part in normalized_parts): + return False + if match.group(0).lower() not in {"access token", "access tokens"}: + return False + + context = get_context(content, match.start()) + if not _PE3_TOKEN_LIFECYCLE_CONTEXT.search(context): + return False + if _PE3_TOKEN_ACTION_CONTEXT.search(context) or _PE3_TOKEN_SENSITIVE_SOURCE.search(context): + return False + + line = _source_line(content, match) + line_start = content.rfind("\n", 0, match.start()) + 1 + relative_start = match.start() - line_start + relative_end = match.end() - line_start + prefix = line[:relative_start] + suffix = line[relative_end:] + + has_noun_modifier = _PE3_ACCESS_TOKEN_NOUN_MODIFIER.search(prefix) is not None + is_lifecycle_subject = bool( + _PE3_ACCESS_TOKEN_LIFESPAN_PREFIX.fullmatch(prefix) + and _PE3_ACCESS_TOKEN_LIFESPAN_SUFFIX.match(suffix) + ) + return has_noun_modifier or is_lifecycle_subject + + def _is_qualified_benign_access_requirement( content: str, match: re.Match[str], file_type: str ) -> bool: @@ -245,7 +321,7 @@ def loc(ln: int) -> Location: for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): line_num = get_line_number(content, match.start()) context = get_context(content, match.start()) - if _is_pe3_documentation_example(content, match, file_type): + if _is_pe3_documentation_example(content, match, file_type, file_path): continue if _is_qualified_benign_access_requirement(content, match, file_type): continue @@ -335,26 +411,35 @@ def _is_documentation_example(context: str, file_type: str) -> bool: return _has_documentation_indicator(context, _DOCUMENTATION_EXAMPLE_INDICATORS) -def _is_pe3_documentation_example(content: str, match: re.Match[str], file_type: str) -> bool: - """Filter only the reviewed, position-bound access-token UI path. +def _is_pe3_documentation_example( + content: str, + match: re.Match[str], + file_type: str, + file_path: str, +) -> bool: + """Filter reviewed, position-bound access-token documentation forms. Generic words such as ``example``, ``documentation``, ``Required``, and ``environment variable`` are attacker-controllable prose and must never suppress an otherwise actionable credential-access match. Even negated references remain findings because another malicious clause can share the - same line. + same line. The OAuth lifecycle exception is separately bounded by noun + grammar, lifecycle evidence, and action/sensitive-source vetoes. """ if file_type not in {"markdown", "text"}: return False - line = _source_line(content, match) if match.group(0).lower() not in {"access token", "access tokens"}: return False + + line = _source_line(content, match) navigation = _PE3_SAFE_ACCESS_TOKEN_NAVIGATION.search(line) - if navigation is None: - return False - line_start = content.rfind("\n", 0, match.start()) + 1 - match_span = (match.start() - line_start, match.end() - line_start) - return navigation.span("target") == match_span + if navigation is not None: + line_start = content.rfind("\n", 0, match.start()) + 1 + match_span = (match.start() - line_start, match.end() - line_start) + if navigation.span("target") == match_span: + return True + + return _is_access_token_lifecycle_noun(content, match, file_type, file_path) def node(state: SkillspectorState) -> AnalyzerNodeResponse: diff --git a/src/skillspector/nodes/build_context.py b/src/skillspector/nodes/build_context.py index 9aa76db8..e0714900 100644 --- a/src/skillspector/nodes/build_context.py +++ b/src/skillspector/nodes/build_context.py @@ -94,6 +94,42 @@ def _resolve_skill_dir(state: SkillspectorState) -> Path: return resolved +def _selected_baseline_component( + state: SkillspectorState, + skill_dir: Path, + inventoried_components: list[str], +) -> str | None: + """Return the selected baseline's component path when it is inside the skill. + + The CLI records the exact path selected by ``scan --baseline`` or targeted + by ``baseline -o``. Excluding only that file prevents a rule's own sensitive + message glob from producing a fresh finding (or entering regenerated + fingerprints) while leaving every sibling YAML/JSON file in normal scope. + """ + raw_path = state.get("baseline_path") + if not isinstance(raw_path, str) or not raw_path.strip(): + return None + + baseline_path = Path(raw_path) + candidates: list[Path] = [baseline_path] + try: + resolved = baseline_path.resolve() + except (OSError, RuntimeError): + resolved = None + if resolved is not None and resolved != baseline_path: + candidates.append(resolved) + + inventory = frozenset(inventoried_components) + for candidate in candidates: + try: + relative = candidate.relative_to(skill_dir).as_posix() + except ValueError: + continue + if relative in inventory: + return relative + return None + + def _walk_skill_files( skill_dir: Path, ) -> tuple[list[str], list[InspectionLedgerEvent]]: @@ -417,7 +453,13 @@ def build_context(state: SkillspectorState) -> dict[str, object]: and _is_valid_oms_signature(skill_dir / _OMS_SIGNATURE_PATH) else set() ) - components = [path for path in inventoried_components if path not in recognized_oms_signatures] + selected_baseline = _selected_baseline_component(state, skill_dir, inventoried_components) + selected_baselines = frozenset({selected_baseline} if selected_baseline else set()) + components = [ + path + for path in inventoried_components + if path not in recognized_oms_signatures and path not in selected_baselines + ] signature_events = [ ledger_event( outcome=LedgerOutcome.OUT_OF_SCOPE, @@ -428,17 +470,35 @@ def build_context(state: SkillspectorState) -> dict[str, object]: ) for path in sorted(recognized_oms_signatures) ] + baseline_events = [ + ledger_event( + outcome=LedgerOutcome.OUT_OF_SCOPE, + record_type=LedgerRecordType.SCOPE_BOUNDARY, + phase="discovery", + path=path, + reason=LedgerReason.BASELINE_FILE, + ) + for path in sorted(selected_baselines) + ] file_cache, cache_events = _read_file_cache(skill_dir, components) python_ast_cache_key = prewarm_python_ast_cache(components, file_cache) manifest = _parse_manifest(skill_dir) + metadata_components = [ + path for path in inventoried_components if path not in selected_baselines + ] component_metadata, has_executable_scripts = _build_component_metadata( - skill_dir, inventoried_components, file_cache, recognized_oms_signatures + skill_dir, metadata_components, file_cache, recognized_oms_signatures ) return { "components": components, "file_cache": file_cache, - "inspection_ledger": [*discovery_events, *signature_events, *cache_events], + "inspection_ledger": [ + *discovery_events, + *signature_events, + *baseline_events, + *cache_events, + ], "ast_cache": {}, "python_ast_cache_key": python_ast_cache_key, "manifest": manifest, diff --git a/src/skillspector/nodes/meta_analyzer.py b/src/skillspector/nodes/meta_analyzer.py index 08093601..70ffe1eb 100644 --- a/src/skillspector/nodes/meta_analyzer.py +++ b/src/skillspector/nodes/meta_analyzer.py @@ -335,7 +335,7 @@ class LLMMetaAnalyzer(LLMAnalyzerBase): response_schema = MetaAnalyzerResult def __init__(self, model: str): - super().__init__(base_prompt=PER_FILE_ANALYSIS_PROMPT, model=model) + super().__init__(base_prompt=PER_FILE_ANALYSIS_PROMPT, model=model, node="meta_analyzer") def _estimate_extra_overhead(self, findings: list[Finding]) -> int: if not findings: @@ -644,6 +644,8 @@ def meta_analyzer(state: SkillspectorState) -> MetaAnalyzerResponse: metadata_text = _format_metadata(manifest) files_with_findings = sorted({f.file for f in findings}) + analyzer: LLMMetaAnalyzer | None = None + batches: list[Batch] = [] try: # Construct inside the try so a chat-model construction failure is caught # and recorded as a degraded LLM call (consistent with the semantic @@ -732,21 +734,34 @@ def meta_analyzer(state: SkillspectorState) -> MetaAnalyzerResponse: ok=bool(detailed.successful) or not detailed.failures, ) ], + "inference_usage": analyzer.inference_usage, } - except ValueError: - raise except Exception as e: + post_response_value_error = ( + isinstance(e, ValueError) and analyzer is not None and analyzer.response_received + ) + if isinstance(e, ValueError) and not post_response_value_error: + raise logger.warning("LLM call failed, passing all findings through (fail-closed): %s", e) filtered = _passthrough_with_defaults(findings) + if post_response_value_error: + ledger_events, status = _meta_ledger_response( + batches, + BatchExecutionResult( + failures=[ + BatchFailure(batch=batch, error_class=type(e).__name__) for batch in batches + ] + ), + filtered, + ) + else: + ledger_events = [] + status = analyzer_status_event(analyzer_id="meta_analyzer", status="unavailable") return { "findings": filtered, "effective_finding_ids": [finding.finding_id for finding in filtered], - "inspection_ledger": [], - "analyzer_status_events": [ - analyzer_status_event( - analyzer_id="meta_analyzer", - status="unavailable", - ) - ], + "inspection_ledger": ledger_events, + "analyzer_status_events": [status], "llm_call_log": [llm_call_record("meta_analyzer", ok=False, error=str(e))], + "inference_usage": analyzer.inference_usage if analyzer is not None else [], } diff --git a/src/skillspector/nodes/report.py b/src/skillspector/nodes/report.py index 61d1f09f..fb7dc552 100644 --- a/src/skillspector/nodes/report.py +++ b/src/skillspector/nodes/report.py @@ -35,6 +35,7 @@ from rich.table import Table from skillspector import __version__ as skillspector_version +from skillspector.inference_usage import sanitize_inference_usage from skillspector.inspection_ledger import AnalysisCompleteness from skillspector.llm_utils import is_llm_available from skillspector.logging_config import get_logger @@ -603,6 +604,7 @@ def _build_metadata( has_executable_scripts: bool, use_llm: bool, llm_call_log: Sequence[Mapping[str, object]] | None = None, + inference_usage: Sequence[Mapping[str, object]] | None = None, ) -> dict[str, object]: """Build the metadata section shared by all output formats.""" llm_call_log = llm_call_log or [] @@ -620,6 +622,10 @@ def _build_metadata( # available AND the stage was not fully degraded (every call failing). "llm_available": llm_available and not degraded, "meta_analysis_applied": meta_analysis_applied, + # A list (including an empty list) makes observability explicit. Empty + # means the provider/transport supplied no counters; it is never an + # estimated zero-cost assertion. + "inference_usage": sanitize_inference_usage(inference_usage), } if not meta_analysis_applied: meta["filtering_mode"] = "heuristic" @@ -652,6 +658,7 @@ def _format_json( has_executable_scripts: bool, use_llm: bool = True, llm_call_log: Sequence[Mapping[str, object]] | None = None, + inference_usage: Sequence[Mapping[str, object]] | None = None, analysis_completeness: Mapping[str, object] | None = None, suppressed: list[SuppressedFinding] | None = None, execution_successful: bool = True, @@ -683,7 +690,12 @@ def _format_json( "issues": [f.to_dict() for f in findings], "suppressed_count": len(suppressed), "suppressed": [sf.to_dict() for sf in suppressed], - "metadata": _build_metadata(has_executable_scripts, use_llm, llm_call_log), + "metadata": _build_metadata( + has_executable_scripts, + use_llm, + llm_call_log, + inference_usage, + ), "execution_successful": execution_successful, } data["analysis_completeness"] = dict(analysis_completeness or {}) @@ -896,6 +908,7 @@ def report(state: SkillspectorState) -> dict[str, object]: output_format = state.get("output_format") or "sarif" use_llm = state.get("use_llm", True) llm_call_log = state.get("llm_call_log") or [] + inference_usage = state.get("inference_usage") or [] _attempted, _succeeded, degraded = _llm_runtime_status(use_llm, llm_call_log) degraded_notice = _llm_degradation_notice(use_llm, llm_call_log) @@ -970,6 +983,7 @@ def report(state: SkillspectorState) -> dict[str, object]: has_executable_scripts, use_llm=use_llm, llm_call_log=llm_call_log, + inference_usage=inference_usage, analysis_completeness=analysis_completeness, suppressed=suppressed, execution_successful=execution_successful, diff --git a/src/skillspector/providers/__init__.py b/src/skillspector/providers/__init__.py index a4c0d709..f380fda1 100644 --- a/src/skillspector/providers/__init__.py +++ b/src/skillspector/providers/__init__.py @@ -197,13 +197,13 @@ def resolve_chat_model_credentials() -> tuple[str, str | None] | None: return _openai_fallback_provider().resolve_credentials() -def create_chat_model( +def create_chat_model_with_provider( model: str, *, max_tokens: int, timeout: float | None = 120, -) -> BaseChatModel: - """Create the active provider's native LangChain chat model. +) -> tuple[BaseChatModel, LLMProvider]: + """Create a chat model and return the provider that actually built it. CLI providers (``claude_cli``, ``codex_cli``, ``gemini_cli``) do not have a native LangChain chat model — callers that need CLI transport should use @@ -220,7 +220,7 @@ def create_chat_model( if not has_cli_capability(provider): llm = provider.create_chat_model(model, max_tokens=max_tokens, timeout=timeout) if llm is not None: - return llm + return llm, provider if has_provider_binding(): raise_no_llm_api_key_configured() @@ -228,17 +228,33 @@ def create_chat_model( from .openai import OpenAIProvider if not isinstance(provider, OpenAIProvider): - llm = _openai_fallback_provider().create_chat_model( + fallback_provider = _openai_fallback_provider() + llm = fallback_provider.create_chat_model( model, max_tokens=max_tokens, timeout=timeout, ) if llm is not None: - return llm + return llm, fallback_provider raise_no_llm_api_key_configured() +def create_chat_model( + model: str, + *, + max_tokens: int, + timeout: float | None = 120, +) -> BaseChatModel: + """Create the active provider's native LangChain chat model.""" + llm, _provider = create_chat_model_with_provider( + model, + max_tokens=max_tokens, + timeout=timeout, + ) + return llm + + __all__ = [ "AgentCLICapable", "ChatModelProvider", @@ -247,6 +263,7 @@ def create_chat_model( "ModelMetadataProvider", "NO_LLM_API_KEY_MESSAGE", "create_chat_model", + "create_chat_model_with_provider", "get_active_provider", "get_metadata_provider", "has_cli_capability", diff --git a/src/skillspector/state.py b/src/skillspector/state.py index 581514ad..f7942bf7 100644 --- a/src/skillspector/state.py +++ b/src/skillspector/state.py @@ -22,6 +22,7 @@ from typing_extensions import TypedDict +from skillspector.inference_usage import InferenceUsageRecord from skillspector.inspection_ledger import ( AnalysisCompleteness, AnalyzerStatusEvent, @@ -87,11 +88,21 @@ class SkillspectorState(TypedDict, total=False): # the parallel analyzer nodes (same pattern as ``findings``). llm_call_log: Annotated[list[LLMCallRecord], operator.add] + # Exact provider-response token counters. Each LLM-backed node appends its + # per-call records; the report exposes the sanitized projection under + # metadata.inference_usage. Missing records mean "not observable", never + # an estimated zero. + inference_usage: Annotated[list[InferenceUsageRecord], operator.add] + # Baseline / false-positive suppression. `baseline` is a loaded # skillspector.suppression.Baseline (set by CLI/API); the report node drops # matching findings before scoring. `show_suppressed` keeps them in the # report (marked) for review; `suppressed_findings` is the report output. baseline: object | None + # Absolute path selected by `scan --baseline` or targeted by `baseline -o`. + # When it is inside the scan target, build_context excludes only that file + # so waiver text cannot scan itself or enter regenerated fingerprints. + baseline_path: str | None show_suppressed: bool suppressed_findings: list[object] @@ -150,6 +161,7 @@ class AnalyzerNodeResponse(TypedDict): # LLM-backed analyzers also report one telemetry record; static analyzers # omit it (NotRequired keeps the key optional for them). llm_call_log: NotRequired[list[LLMCallRecord]] + inference_usage: NotRequired[list[InferenceUsageRecord]] class MetaAnalyzerResponse(TypedDict): @@ -160,3 +172,4 @@ class MetaAnalyzerResponse(TypedDict): inspection_ledger: NotRequired[list[InspectionLedgerEvent]] analyzer_status_events: NotRequired[list[AnalyzerStatusEvent]] llm_call_log: NotRequired[list[LLMCallRecord]] + inference_usage: NotRequired[list[InferenceUsageRecord]] diff --git a/tests/nodes/analyzers/test_semantic_security_discovery.py b/tests/nodes/analyzers/test_semantic_security_discovery.py index 4ce12117..b0513e3a 100644 --- a/tests/nodes/analyzers/test_semantic_security_discovery.py +++ b/tests/nodes/analyzers/test_semantic_security_discovery.py @@ -330,6 +330,27 @@ def test_generic_exception_returns_empty(self, mock_get_model: MagicMock) -> Non assert status["status"] == "unavailable" assert "reason_code" not in status + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_generic_exception_preserves_missing_cache_events(self) -> None: + from skillspector.llm_analyzer_base import LLMAnalyzerBase + + with patch.object( + LLMAnalyzerBase, + "run_batches", + side_effect=RuntimeError("LLM service unavailable"), + ): + result = node( + { + "components": ["cached.py", "missing.py"], + "file_cache": {"cached.py": "print('ready')\n"}, + } + ) + + assert [(event["path"], event["reason_code"]) for event in result["inspection_ledger"]] == [ + ("missing.py", "missing_file_cache") + ] + assert result["analyzer_status_events"][0]["status"] == "unavailable" + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) def test_validation_error_returns_empty(self) -> None: """Malformed LLM response (ValidationError) must not crash the graph.""" diff --git a/tests/nodes/test_llm_analyzer_base.py b/tests/nodes/test_llm_analyzer_base.py index 1d9b6a23..6a081521 100644 --- a/tests/nodes/test_llm_analyzer_base.py +++ b/tests/nodes/test_llm_analyzer_base.py @@ -22,6 +22,7 @@ import pytest from langchain_core.messages import AIMessage +from pydantic import ValidationError from skillspector.inspection_ledger import LedgerReason, finalize_ledger from skillspector.llm_analyzer_base import ( @@ -39,6 +40,7 @@ number_lines, resolve_max_concurrency, ) +from skillspector.llm_utils import AgentCLIChatModel from skillspector.models import Finding from skillspector.nodes.meta_analyzer import ( LLMMetaAnalyzer, @@ -194,6 +196,13 @@ def _mock_get_chat_model(*_args, **_kwargs): MOCK_PATCH_TARGET = "skillspector.llm_analyzer_base.get_chat_model" +def _structured_response_validation_error() -> ValidationError: + """Build the error raised when a provider returns malformed findings.""" + with pytest.raises(ValidationError) as exc_info: + LLMAnalysisResult.model_validate({"findings": "not-an-array"}) + return exc_info.value + + class _RawTextAnalyzer(LLMAnalyzerBase): """Test analyzer for raw-string mode.""" @@ -404,6 +413,18 @@ def test_run_batches_uses_message_text_for_content_blocks(self) -> None: assert results[0][1] == ["chunk"] + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_run_batches_uses_agent_cli_message_content(self) -> None: + analyzer = _RawTextAnalyzer(base_prompt="test", model=self.MODEL) + provider = MagicMock() + provider.complete.return_value = "raw CLI response" + analyzer._llm = AgentCLIChatModel(provider, self.MODEL, 1024) + + results = analyzer.run_batches([Batch(file_path="a.py", content="code")]) + + assert results[0][1] == ["raw CLI response"] + provider.complete.assert_called_once() + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) async def test_arun_batches_uses_message_text_for_content_blocks(self) -> None: analyzer = _RawTextAnalyzer(base_prompt="test", model=self.MODEL) @@ -415,6 +436,100 @@ async def test_arun_batches_uses_message_text_for_content_blocks(self) -> None: assert results[0][1] == ["async chunk"] + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + async def test_arun_batches_uses_agent_cli_message_content(self) -> None: + analyzer = _RawTextAnalyzer(base_prompt="test", model=self.MODEL) + provider = MagicMock() + provider.complete.return_value = "async raw CLI response" + analyzer._llm = AgentCLIChatModel(provider, self.MODEL, 1024) + + results = await analyzer.arun_batches([Batch(file_path="a.py", content="code")]) + + assert results[0][1] == ["async raw CLI response"] + provider.complete.assert_called_once() + + +# --------------------------------------------------------------------------- +# LLMAnalyzerBase.run_batches (sync sequential execution) +# --------------------------------------------------------------------------- + + +class TestRunBatches: + MODEL = "nvidia/openai/gpt-oss-120b" + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_structured_validation_error_recovers_on_retry(self) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.invoke = MagicMock( + side_effect=[ + _structured_response_validation_error(), + LLMAnalysisResult(findings=[]), + ] + ) + batch = Batch(file_path="a.py", content="code") + + outcome = analyzer.run_batches_detailed([batch]) + + assert [item[0].file_path for item in outcome.successful] == ["a.py"] + assert outcome.failures == [] + assert analyzer._structured_llm.invoke.call_count == 2 + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_structured_validation_error_isolated_after_retry(self) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.invoke = MagicMock( + side_effect=[ + _structured_response_validation_error(), + _structured_response_validation_error(), + LLMAnalysisResult(findings=[]), + ] + ) + batches = [ + Batch(file_path="malformed.py", content="bad response"), + Batch(file_path="clean.py", content="clean response"), + ] + + outcome = analyzer.run_batches_detailed(batches) + + assert [item[0].file_path for item in outcome.successful] == ["clean.py"] + assert [(failure.batch.file_path, failure.error_class) for failure in outcome.failures] == [ + ("malformed.py", "ValidationError") + ] + assert analyzer._structured_llm.invoke.call_count == 3 + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_value_error_still_propagates_without_retry(self) -> None: + """Non-validation ValueError instances still signal misconfiguration.""" + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.invoke = MagicMock(side_effect=ValueError("no API key")) + + with pytest.raises(ValueError, match="no API key"): + analyzer.run_batches_detailed([Batch(file_path="a.py", content="code")]) + + analyzer._structured_llm.invoke.assert_called_once() + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_custom_parser_validation_error_propagates_without_retry(self) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.invoke = MagicMock(return_value=LLMAnalysisResult(findings=[])) + analyzer.parse_response = MagicMock(side_effect=_structured_response_validation_error()) + + with pytest.raises(ValidationError): + analyzer.run_batches_detailed([Batch(file_path="a.py", content="code")]) + + analyzer._structured_llm.invoke.assert_called_once() + analyzer.parse_response.assert_called_once() + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_prompt_validation_error_propagates_without_invoke(self) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer.build_prompt = MagicMock(side_effect=_structured_response_validation_error()) + + with pytest.raises(ValidationError): + analyzer.run_batches_detailed([Batch(file_path="a.py", content="code")]) + + analyzer._structured_llm.invoke.assert_not_called() + # --------------------------------------------------------------------------- # LLMAnalyzerBase.arun_batches (async parallel execution) @@ -462,6 +577,68 @@ async def test_detailed_outcome_preserves_failed_batch(self) -> None: assert outcome.failures[0].batch.file_path == "b.py" assert outcome.failures[0].error_class == "TimeoutError" + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + async def test_structured_validation_error_recovers_on_retry(self) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.ainvoke = AsyncMock( + side_effect=[ + _structured_response_validation_error(), + LLMAnalysisResult(findings=[]), + ] + ) + batch = Batch(file_path="a.py", content="code") + + outcome = await analyzer.arun_batches_detailed([batch]) + + assert [item[0].file_path for item in outcome.successful] == ["a.py"] + assert outcome.failures == [] + assert analyzer._structured_llm.ainvoke.call_count == 2 + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + async def test_structured_validation_error_isolated_after_retry(self) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.ainvoke = AsyncMock( + side_effect=[ + _structured_response_validation_error(), + _structured_response_validation_error(), + LLMAnalysisResult(findings=[]), + ] + ) + batches = [ + Batch(file_path="malformed.py", content="bad response"), + Batch(file_path="clean.py", content="clean response"), + ] + + outcome = await analyzer.arun_batches_detailed(batches, max_concurrency=1) + + assert [item[0].file_path for item in outcome.successful] == ["clean.py"] + assert [(failure.batch.file_path, failure.error_class) for failure in outcome.failures] == [ + ("malformed.py", "ValidationError") + ] + assert analyzer._structured_llm.ainvoke.call_count == 3 + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + async def test_custom_parser_validation_error_propagates_without_retry(self) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.ainvoke = AsyncMock(return_value=LLMAnalysisResult(findings=[])) + analyzer.parse_response = MagicMock(side_effect=_structured_response_validation_error()) + + with pytest.raises(ValidationError): + await analyzer.arun_batches_detailed([Batch(file_path="a.py", content="code")]) + + analyzer._structured_llm.ainvoke.assert_awaited_once() + analyzer.parse_response.assert_called_once() + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + async def test_prompt_validation_error_propagates_without_invoke(self) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer.build_prompt = MagicMock(side_effect=_structured_response_validation_error()) + + with pytest.raises(ValidationError): + await analyzer.arun_batches_detailed([Batch(file_path="a.py", content="code")]) + + analyzer._structured_llm.ainvoke.assert_not_called() + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) async def test_returns_parsed_findings(self) -> None: analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) diff --git a/tests/nodes/test_report.py b/tests/nodes/test_report.py index 333b0906..74dff645 100644 --- a/tests/nodes/test_report.py +++ b/tests/nodes/test_report.py @@ -775,6 +775,51 @@ def test_report_not_degraded_when_no_llm_calls(monkeypatch: pytest.MonkeyPatch) assert "llm_calls_attempted" not in meta +def test_json_report_exposes_only_sanitized_provider_usage( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr("skillspector.nodes.report.is_llm_available", lambda: (True, None)) + state: SkillspectorState = { + "filtered_findings": [], + "component_metadata": [], + "has_executable_scripts": False, + "manifest": {}, + "output_format": "json", + "use_llm": True, + "llm_call_log": [llm_call_record("meta_analyzer", ok=True)], + "inference_usage": [ + { + "node": "meta_analyzer", + "request_kind": "structured_output", + "provider": "anthropic", + "model": "claude-sonnet-4-6", + "model_source": "provider_response", + "usage_source": "provider_response", + "prompt_tokens": 123, + "completion_tokens": 45, + "total_tokens": 168, + "secret": "not serialized", + } + ], + } + + meta = _meta_from_json_report(state) + + assert meta["inference_usage"] == [ + { + "node": "meta_analyzer", + "request_kind": "structured_output", + "provider": "anthropic", + "model": "claude-sonnet-4-6", + "model_source": "provider_response", + "usage_source": "provider_response", + "prompt_tokens": 123, + "completion_tokens": 45, + "total_tokens": 168, + } + ] + + def test_report_no_llm_failures_not_counted_as_degraded(monkeypatch: pytest.MonkeyPatch) -> None: """use_llm False -> failures (if any) never mark the scan degraded.""" monkeypatch.setattr("skillspector.nodes.report.is_llm_available", lambda: (True, None)) diff --git a/tests/nodes/test_semantic_quality_policy.py b/tests/nodes/test_semantic_quality_policy.py index e8ba916c..ba294f49 100644 --- a/tests/nodes/test_semantic_quality_policy.py +++ b/tests/nodes/test_semantic_quality_policy.py @@ -21,8 +21,13 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from langchain_core.messages import AIMessage +from langchain_core.outputs import ChatGeneration, LLMResult +from langchain_core.runnables import Runnable, RunnableConfig +from skillspector.inspection_ledger import finalize_ledger from skillspector.llm_analyzer_base import LLMAnalysisResult, LLMFinding +from skillspector.llm_utils import AgentCLIChatModel from skillspector.models import Finding from skillspector.nodes.analyzers.semantic_quality_policy import ( ANALYZER_ID, @@ -59,6 +64,34 @@ def _mock_get_chat_model(*_args, **_kwargs): ) +class _PostResponseValueErrorRunnable(Runnable[str, object]): + """Emit a real callback response before simulating structured parsing failure.""" + + def __init__(self, response: LLMResult) -> None: + self._response = response + + def _raise_after_response(self, config: RunnableConfig | None) -> object: + for callback in (config or {}).get("callbacks", []): + callback.on_llm_end(self._response) + raise ValueError("structured output parse failed") + + def invoke( + self, + input: str, + config: RunnableConfig | None = None, + **kwargs: object, + ) -> object: + return self._raise_after_response(config) + + async def ainvoke( + self, + input: str, + config: RunnableConfig | None = None, + **kwargs: object, + ) -> object: + return self._raise_after_response(config) + + # --------------------------------------------------------------------------- # use_llm guard # --------------------------------------------------------------------------- @@ -261,6 +294,55 @@ def test_generic_exception_returns_empty(self, mock_get_model: MagicMock) -> Non assert status["status"] == "unavailable" assert "reason_code" not in status + def test_post_response_value_error_preserves_provider_usage(self) -> None: + message = AIMessage( + content="malformed structured response", + response_metadata={"model_name": "azure/anthropic/claude-opus-4-6"}, + usage_metadata={ + "input_tokens": 10, + "output_tokens": 2, + "total_tokens": 12, + }, + ) + response = LLMResult( + generations=[[ChatGeneration(message=message)]], + llm_output={}, + ) + mock_llm = MagicMock() + mock_llm.with_structured_output.return_value = _PostResponseValueErrorRunnable(response) + + with patch(MOCK_PATCH_TARGET, return_value=mock_llm): + result = node({"file_cache": {"SKILL.md": "# Skill"}}) + + assert result["findings"] == [] + assert result["inference_usage"][0]["prompt_tokens"] == 10 + assert result["inference_usage"][0]["completion_tokens"] == 2 + assert result["llm_call_log"][0]["ok"] is False + assert result["inspection_ledger"] + assert result["analyzer_status_events"][0]["status"] == "failed" + completeness, _ = finalize_ledger( + { + "components": ["SKILL.md"], + "findings": [], + "inspection_ledger": result["inspection_ledger"], + "analyzer_status_events": result["analyzer_status_events"], + } + ) + assert completeness["execution_successful"] is False + + def test_post_response_value_error_without_usage_uses_failed_fallback(self) -> None: + provider = MagicMock() + provider.complete.return_value = "not valid structured JSON" + cli_model = AgentCLIChatModel(provider, "gpt-5.6-sol", 1024) + + with patch(MOCK_PATCH_TARGET, return_value=cli_model): + result = node({"file_cache": {"SKILL.md": "# Skill"}}) + + assert result["findings"] == [] + assert result["inference_usage"] == [] + assert result["inspection_ledger"] + assert result["analyzer_status_events"][0]["status"] == "failed" + # --------------------------------------------------------------------------- # LLM call telemetry (llm_call_log; drives the report's degradation signal) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 93d9dc83..fb7061f6 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -327,6 +327,135 @@ def test_cli_baseline_generate_then_scan_round_trip(tmp_path: Path) -> None: assert data["risk_assessment"]["score"] == 0 +def test_cli_baseline_regeneration_excludes_in_tree_output(tmp_path: Path) -> None: + """Regeneration cannot fingerprint findings created by the old output file.""" + skill = tmp_path / "skill" + baseline_file = skill / "config" / "skillspector-baseline.yaml" + baseline_file.parent.mkdir(parents=True) + (skill / "SKILL.md").write_text( + "---\nname: regenerate-baseline\n---\nUse --privileged for required device access.\n", + encoding="utf-8", + ) + baseline_file.write_text( + "version: 2\n" + "rules:\n" + " - id: PE5\n" + " path: SKILL.md\n" + ' message: "*--privileged*"\n' + " reason: reviewed device access\n" + "fingerprints: []\n", + encoding="utf-8", + ) + + result = runner.invoke( + app, + [ + "baseline", + str(skill), + "--no-llm", + "--output", + str(baseline_file), + ], + ) + + assert result.exit_code == 0, result.output + generated = yaml.safe_load(baseline_file.read_text(encoding="utf-8")) + assert [entry["rule_id"] for entry in generated["fingerprints"]] == ["PE5"] + assert [entry["file"] for entry in generated["fingerprints"]] == ["SKILL.md"] + + +def test_cli_scan_excludes_selected_baseline_inside_skill(tmp_path: Path) -> None: + """A selected in-tree baseline cannot create findings from its own rule text.""" + skill = tmp_path / "skill" + baseline_file = skill / "config" / "skillspector-baseline.yaml" + baseline_file.parent.mkdir(parents=True) + (skill / "SKILL.md").write_text( + "---\nname: in-tree-baseline\n---\nUse --privileged for required device access.\n", + encoding="utf-8", + ) + baseline_file.write_text( + "version: 2\n" + "rules:\n" + " - id: PE5\n" + " path: SKILL.md\n" + ' message: "*--privileged*"\n' + " reason: reviewed device access\n" + "fingerprints: []\n", + encoding="utf-8", + ) + + result = runner.invoke( + app, + [ + "scan", + str(skill), + "--no-llm", + "--format", + "json", + "--baseline", + str(baseline_file), + ], + ) + + assert result.exit_code == 0, result.output + data = json.loads(result.output) + assert data["issues"] == [] + assert [finding["id"] for finding in data["suppressed"]] == ["PE5"] + assert data["suppressed"][0]["location"]["file"] == "SKILL.md" + assert all( + component["path"] != "config/skillspector-baseline.yaml" for component in data["components"] + ) + assert any( + exclusion["path"] == "config/skillspector-baseline.yaml" + and exclusion["reason_code"] == "baseline_file" + for exclusion in data["analysis_completeness"]["scope_exclusions"] + ) + + +def test_cli_scan_excludes_only_the_selected_baseline(tmp_path: Path) -> None: + """Sibling files remain in scope even when their content resembles a baseline.""" + skill = tmp_path / "skill" + config = skill / "config" + config.mkdir(parents=True) + (skill / "SKILL.md").write_text( + "---\nname: selected-baseline-only\n---\n# Safe skill\n", + encoding="utf-8", + ) + baseline_file = config / "skillspector-baseline.yaml" + baseline_file.write_text( + "version: 2\n" + "rules:\n" + " - id: PE5\n" + " path: SKILL.md\n" + ' message: "*--privileged*"\n' + " reason: reviewed device access\n" + "fingerprints: []\n", + encoding="utf-8", + ) + (config / "review.yaml").write_text("flag: --privileged\n", encoding="utf-8") + + result = runner.invoke( + app, + [ + "scan", + str(skill), + "--no-llm", + "--format", + "json", + "--baseline", + str(baseline_file), + ], + ) + + assert result.exit_code in {0, 1}, result.output + data = json.loads(result.output) + pe5_files = { + finding["location"]["file"] for finding in data["issues"] if finding["id"] == "PE5" + } + assert pe5_files == {"config/review.yaml"} + assert data["suppressed_count"] == 0 + + def test_recursive_multi_skill_scan_rejects_shared_baseline(tmp_path: Path) -> None: """Exact baselines are per-skill and cannot be silently reused recursively.""" root = tmp_path / "skills" diff --git a/tests/unit/test_inference_usage.py b/tests/unit/test_inference_usage.py new file mode 100644 index 00000000..72b18280 --- /dev/null +++ b/tests/unit/test_inference_usage.py @@ -0,0 +1,419 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Provider-response inference usage normalization tests.""" + +from __future__ import annotations + +from types import SimpleNamespace + +from langchain_core.messages import AIMessage +from langchain_core.outputs import ChatGeneration, LLMResult + +from skillspector.inference_usage import ( + InferenceUsageCollector, + _usage_record, + sanitize_inference_usage, +) + + +def test_collector_captures_standardized_langchain_usage_without_double_counting_cache() -> None: + """LangChain input_tokens is already inclusive of its cache partitions.""" + message = AIMessage( + content="ok", + response_metadata={"model_name": "claude-opus-4-8-20260801"}, + usage_metadata={ + "input_tokens": 100, + "output_tokens": 20, + "total_tokens": 120, + "input_token_details": {"cache_read": 60, "cache_creation": 10}, + "output_token_details": {"reasoning": 5}, + }, + ) + collector = InferenceUsageCollector( + node="semantic_security_discovery", + request_kind="structured_output", + provider="anthropic", + requested_model="claude-opus-4-8", + ) + + collector.on_llm_end(LLMResult(generations=[[ChatGeneration(message=message)]], llm_output={})) + + assert collector.snapshot() == [ + { + "node": "semantic_security_discovery", + "request_kind": "structured_output", + "provider": "anthropic", + "model": "claude-opus-4-8-20260801", + "model_source": "provider_response", + "usage_source": "provider_response", + "prompt_tokens": 100, + "completion_tokens": 20, + "cached_tokens": 60, + "cache_write_tokens": 10, + "reasoning_tokens": 5, + "total_tokens": 120, + } + ] + + +def test_collector_marks_response_received_without_usage_counters() -> None: + collector = InferenceUsageCollector( + node="semantic_quality_policy", + request_kind="structured_output", + provider="codex_cli", + requested_model="gpt-5.6-sol", + ) + message = AIMessage(content="provider returned without usage metadata") + + collector.on_llm_end(LLMResult(generations=[[ChatGeneration(message=message)]], llm_output={})) + + assert collector.response_received is True + assert collector.snapshot() == [] + + +def test_raw_anthropic_usage_adds_external_cache_counters_to_prompt_total() -> None: + """Anthropic raw input_tokens excludes cache reads and cache creation.""" + message = SimpleNamespace( + usage_metadata=None, + response_metadata={ + "model": "claude-sonnet-4-6", + "usage": { + "input_tokens": 30, + "output_tokens": 7, + "cache_read_input_tokens": 50, + "cache_creation_input_tokens": 20, + }, + }, + ) + + record = _usage_record( + message, + {}, + node="meta_analyzer", + request_kind="structured_output", + provider="anthropic", + requested_model="claude-sonnet-4-6", + ) + + assert record is not None + assert record["prompt_tokens"] == 100 + assert record["completion_tokens"] == 7 + assert record["cached_tokens"] == 50 + assert record["cache_write_tokens"] == 20 + assert record["total_tokens"] == 107 + + +def test_raw_anthropic_ttl_cache_writes_are_included_in_prompt_total() -> None: + """Raw TTL partitions are direct cache writes even without a generic total.""" + message = SimpleNamespace( + usage_metadata=None, + response_metadata={ + "model": "claude-sonnet-4-6", + "usage": { + "input_tokens": 85, + "output_tokens": 7, + "cache_creation": { + "ephemeral_5m_input_tokens": 10, + "ephemeral_1h_input_tokens": 5, + }, + }, + }, + ) + + record = _usage_record( + message, + {}, + node="meta_analyzer", + request_kind="structured_output", + provider="anthropic", + requested_model="claude-sonnet-4-6", + ) + + assert record is not None + assert record["prompt_tokens"] == 100 + assert record["completion_tokens"] == 7 + assert record["cache_write_tokens"] == 15 + assert record["total_tokens"] == 107 + + +def test_standardized_prompt_wins_when_raw_anthropic_cache_usage_is_also_present() -> None: + """A LangChain AIMessage can carry both normalized and raw usage views.""" + message = SimpleNamespace( + usage_metadata={ + "input_tokens": 100, + "output_tokens": 7, + "total_tokens": 107, + }, + response_metadata={ + "model": "claude-sonnet-4-6", + "usage": { + "input_tokens": 30, + "output_tokens": 7, + "cache_read_input_tokens": 50, + "cache_creation_input_tokens": 20, + }, + }, + ) + + record = _usage_record( + message, + {}, + node="meta_analyzer", + request_kind="structured_output", + provider="anthropic", + requested_model="claude-sonnet-4-6", + ) + + assert record is not None + assert record["prompt_tokens"] == 100 + assert record["cached_tokens"] == 50 + assert record["cache_write_tokens"] == 20 + assert record["total_tokens"] == 107 + + +def test_anthropic_ttl_cache_creation_partitions_override_zero_generic_counter() -> None: + """LangChain exposes 5m/1h writes separately and zeros the generic field.""" + message = SimpleNamespace( + usage_metadata={ + "input_tokens": 100, + "output_tokens": 7, + "total_tokens": 107, + "input_token_details": { + "cache_creation": 0, + "ephemeral_5m_input_tokens": 10, + "ephemeral_1h_input_tokens": 5, + }, + }, + response_metadata={ + "model": "claude-sonnet-4-6", + "usage": { + "input_tokens": 85, + "output_tokens": 7, + "cache_creation_input_tokens": 15, + "cache_creation": { + "ephemeral_5m_input_tokens": 10, + "ephemeral_1h_input_tokens": 5, + }, + }, + }, + ) + + record = _usage_record( + message, + {}, + node="meta_analyzer", + request_kind="structured_output", + provider="anthropic", + requested_model="claude-sonnet-4-6", + ) + + assert record is not None + assert record["prompt_tokens"] == 100 + assert record["cache_write_tokens"] == 15 + assert record["total_tokens"] == 107 + + +def test_openai_nested_cached_and_reasoning_counters_are_subsets() -> None: + message = SimpleNamespace( + usage_metadata=None, + response_metadata={ + "model_name": "gpt-5.6-sol", + "token_usage": { + "prompt_tokens": 90, + "completion_tokens": 12, + "total_tokens": 102, + "prompt_tokens_details": {"cached_tokens": 40}, + "completion_tokens_details": {"reasoning_tokens": 8}, + }, + }, + ) + + record = _usage_record( + message, + {}, + node="semantic_quality_policy", + request_kind="structured_output", + provider="openai", + requested_model="gpt-5.6-sol", + ) + + assert record is not None + assert record["prompt_tokens"] == 90 + assert record["cached_tokens"] == 40 + assert record["reasoning_tokens"] == 8 + assert record["total_tokens"] == 102 + + +def test_standardized_bedrock_total_is_recomputed_from_normalized_partitions() -> None: + message = SimpleNamespace( + usage_metadata={ + "input_tokens": 100, + "output_tokens": 7, + "total_tokens": 92, + "input_token_details": {"cache_read": 15}, + }, + response_metadata={ + "model": "us.anthropic.claude-sonnet-4-6-20250915-v1:0", + }, + ) + + record = _usage_record( + message, + {}, + node="meta_analyzer", + request_kind="structured_output", + provider="bedrock", + requested_model="us.anthropic.claude-sonnet-4-6-20250915-v1:0", + ) + + assert record is not None + assert record["prompt_tokens"] == 100 + assert record["completion_tokens"] == 7 + assert record["cached_tokens"] == 15 + assert record["total_tokens"] == 107 + + +def test_no_provider_counters_produces_no_record() -> None: + message = SimpleNamespace(usage_metadata=None, response_metadata={"model": "some-model"}) + assert ( + _usage_record( + message, + {}, + node="meta_analyzer", + request_kind="structured_output", + provider="nv_inference", + requested_model="some-model", + ) + is None + ) + + +def test_requested_model_fallback_is_explicit_when_response_omits_model() -> None: + message = SimpleNamespace( + usage_metadata={"input_tokens": 4, "output_tokens": 1, "total_tokens": 5}, + response_metadata={}, + ) + + record = _usage_record( + message, + {}, + node="semantic_quality_policy", + request_kind="structured_output", + provider="nv_inference", + requested_model="azure/anthropic/claude-opus-4-6", + ) + + assert record is not None + assert record["model"] == "azure/anthropic/claude-opus-4-6" + assert record["model_source"] == "requested_model" + + +def test_configured_model_echo_is_conservatively_marked_as_requested() -> None: + message = SimpleNamespace( + usage_metadata={"input_tokens": 4, "output_tokens": 1, "total_tokens": 5}, + response_metadata={"model_name": "gpt-5.4"}, + ) + + record = _usage_record( + message, + {"model_name": "gpt-5.4"}, + node="semantic_quality_policy", + request_kind="structured_output", + provider="openai", + requested_model="gpt-5.4", + ) + + assert record is not None + assert record["model"] == "gpt-5.4" + assert record["model_source"] == "requested_model" + + +def test_provider_model_url_with_userinfo_falls_back_to_requested_model() -> None: + message = SimpleNamespace( + usage_metadata={"input_tokens": 4, "output_tokens": 1, "total_tokens": 5}, + response_metadata={"model": "https://key@private-host/v1"}, + ) + + record = _usage_record( + message, + {}, + node="semantic_quality_policy", + request_kind="structured_output", + provider="nv_inference", + requested_model="azure/anthropic/claude-opus-4-6", + ) + + assert record is not None + assert record["model"] == "azure/anthropic/claude-opus-4-6" + assert record["model_source"] == "requested_model" + + +def test_report_sanitizer_rejects_url_and_userinfo_model_labels() -> None: + common = { + "node": "meta_analyzer", + "request_kind": "structured_output", + "provider": "anthropic", + "model_source": "provider_response", + "usage_source": "provider_response", + "prompt_tokens": 11, + } + + assert ( + sanitize_inference_usage( + [ + {**common, "model": "https://key@private-host/v1"}, + {**common, "model": "key@private-host"}, + ] + ) + == [] + ) + + +def test_report_sanitizer_whitelists_fields_and_rejects_invalid_values() -> None: + assert sanitize_inference_usage( + [ + "not-a-record", + { + "node": "meta_analyzer", + "request_kind": "structured_output", + "provider": "anthropic", + "model": "bad\nmodel", + "model_source": "requested_model", + "prompt_tokens": 11, + "completion_tokens": -1, + "api_key": "must-not-leak", + "usage_source": "untrusted", + }, + { + "node": "meta_analyzer", + "request_kind": "structured_output", + "provider": "anthropic", + "model": "claude-sonnet-4-6", + "model_source": "provider_response", + "prompt_tokens": 11, + "completion_tokens": -1, + "api_key": "must-not-leak", + "usage_source": "provider_response", + }, + { + "node": "meta_analyzer", + "request_kind": "structured_output", + "provider": "anthropic", + "model": "claude-sonnet-4-6", + "model_source": "provider_response", + "prompt_tokens": 1 << 63, + "usage_source": "provider_response", + }, + ] + ) == [ + { + "node": "meta_analyzer", + "request_kind": "structured_output", + "provider": "anthropic", + "model": "claude-sonnet-4-6", + "model_source": "provider_response", + "usage_source": "provider_response", + "prompt_tokens": 11, + } + ] diff --git a/tests/unit/test_llm_utils.py b/tests/unit/test_llm_utils.py index 92609337..b9ca2bd5 100644 --- a/tests/unit/test_llm_utils.py +++ b/tests/unit/test_llm_utils.py @@ -28,17 +28,23 @@ import pytest from langchain_anthropic import ChatAnthropic from langchain_core.messages import AIMessage +from langchain_core.outputs import ChatGeneration, LLMResult from pydantic import BaseModel from skillspector import llm_utils +from skillspector.inference_usage import InferenceUsageCollector from skillspector.llm_utils import ( AgentCLIChatModel, + _ainvoke_with_usage, _extract_json_object, + _invoke_with_usage, _resolve_llm_credentials, chat_completion, + chat_model_provider_name, fetch_model_token_limits, get_chat_model, is_llm_available, + new_inference_usage_collector, run_async, ) from skillspector.providers import ( @@ -430,6 +436,80 @@ class _Schema(BaseModel): "x" ) + def test_structured_usage_marks_response_before_sync_parse_failure(self) -> None: + class _Schema(BaseModel): + verdict: str + + provider = MagicMock() + provider.complete.return_value = "not structured JSON" + runnable = AgentCLIChatModel(provider, "claude-sonnet-4-6", 1024).with_structured_output( + _Schema + ) + collector = InferenceUsageCollector( + node="semantic_quality_policy", + request_kind="structured_output", + provider="claude_cli", + requested_model="claude-sonnet-4-6", + ) + + with pytest.raises(ValueError, match="JSON"): + _invoke_with_usage(runnable, "prompt", collector) + + assert collector.response_received is True + assert collector.snapshot() == [] + + async def test_concurrent_structured_usage_marks_each_async_response(self) -> None: + class _Schema(BaseModel): + verdict: str + + provider = MagicMock() + provider.complete.return_value = "not structured JSON" + runnable = AgentCLIChatModel(provider, "claude-sonnet-4-6", 1024).with_structured_output( + _Schema + ) + collectors = [ + InferenceUsageCollector( + node=f"semantic_quality_policy_{index}", + request_kind="structured_output", + provider="claude_cli", + requested_model="claude-sonnet-4-6", + ) + for index in range(2) + ] + + results = await asyncio.gather( + *( + _ainvoke_with_usage(runnable, f"prompt-{index}", collector) + for index, collector in enumerate(collectors) + ), + return_exceptions=True, + ) + + assert all(isinstance(result, ValueError) for result in results) + assert all(collector.response_received for collector in collectors) + assert all(collector.snapshot() == [] for collector in collectors) + + def test_structured_usage_does_not_mark_pre_response_transport_failure(self) -> None: + class _Schema(BaseModel): + verdict: str + + provider = MagicMock() + provider.complete.side_effect = RuntimeError("CLI process failed") + runnable = AgentCLIChatModel(provider, "claude-sonnet-4-6", 1024).with_structured_output( + _Schema + ) + collector = InferenceUsageCollector( + node="semantic_quality_policy", + request_kind="structured_output", + provider="claude_cli", + requested_model="claude-sonnet-4-6", + ) + + with pytest.raises(RuntimeError, match="CLI process failed"): + _invoke_with_usage(runnable, "prompt", collector) + + assert collector.response_received is False + class TestExtractJsonObject: def test_plain_json(self) -> None: @@ -447,6 +527,36 @@ def test_garbage_raises(self) -> None: class TestGetChatModel: + def test_bedrock_dispatch_remains_telemetry_provider_with_openai_key( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("SKILLSPECTOR_PROVIDER", "bedrock") + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai") + fake_model = MagicMock() + + with patch( + "skillspector.providers.bedrock.provider.BedrockProvider.create_chat_model", + return_value=fake_model, + ): + chat_model = get_chat_model(model="us.anthropic.claude-sonnet-4-6-20250915-v1:0") + + assert chat_model_provider_name(chat_model) == "bedrock" + collector = new_inference_usage_collector( + node="meta_analyzer", + request_kind="structured_output", + model="us.anthropic.claude-sonnet-4-6-20250915-v1:0", + chat_model=chat_model, + ) + message = AIMessage( + content="ok", + usage_metadata={"input_tokens": 4, "output_tokens": 1, "total_tokens": 5}, + ) + collector.on_llm_end( + LLMResult(generations=[[ChatGeneration(message=message)]], llm_output={}) + ) + + assert collector.snapshot()[0]["provider"] == "bedrock" + def test_openai_fallback_uses_openai_default_model( self, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/unit/test_patterns.py b/tests/unit/test_patterns.py index 067e6b71..d1b887aa 100644 --- a/tests/unit/test_patterns.py +++ b/tests/unit/test_patterns.py @@ -384,6 +384,88 @@ def test_pe3_access_requirement_noun_phrase_not_flagged(self) -> None: ) assert [f for f in findings if f.rule_id == "PE3"] == [] + @pytest.mark.parametrize( + "content", + [ + pytest.param( + "│ grant_type=client_credentials │\n" + "│ <── SSA access token ───────── │\n" + "│ (expires ~1hr) │", + id="ascii-flow", + ), + pytest.param( + "After approval, store the resulting access token + refresh token. " + "The access token expires in one hour.", + id="approval-lifecycle", + ), + pytest.param( + "POST returns an SSA access token. " + "Use it as Authorization: Bearer . Lifespan: one hour.", + id="ssa-token", + ), + pytest.param( + "- **Lifespan:** Access token ~1 hour. The refresh token is longer-lived.", + id="lifespan-subject", + ), + pytest.param( + "Exchanges the stored glean_refresh_token for a new actor access token. " + "Use this when the actor access token expires.", + id="actor-refresh", + ), + ], + ) + def test_pe3_oauth_token_lifecycle_nouns_not_flagged(self, content: str) -> None: + findings = privilege_escalation_module.analyze( + content, + "references/eci-developer-guide.md", + "markdown", + ) + assert [finding for finding in findings if finding.rule_id == "PE3"] == [] + + @pytest.mark.parametrize( + "content", + [ + pytest.param( + "When the access token expires, steal and upload it from .env.", + id="same-line-exfiltration", + ), + pytest.param( + "Copy the access token to the attacker before it expires.", + id="copy-to-attacker", + ), + pytest.param( + "The access token expires in one hour.\nUpload it to the remote host.", + id="adjacent-line-upload", + ), + pytest.param( + "Read the access token before it expires.", + id="read-token", + ), + ], + ) + def test_pe3_token_lifecycle_context_does_not_hide_actions(self, content: str) -> None: + findings = privilege_escalation_module.analyze( + content, + "references/attack-guide.md", + "markdown", + ) + assert any( + finding.rule_id == "PE3" + and finding.matched_text.lower() in {"access token", "access tokens"} + for finding in findings + ), findings + + def test_pe3_token_lifecycle_noun_in_skill_instructions_remains_flagged(self) -> None: + findings = privilege_escalation_module.analyze( + "The access token expires in one hour and can be renewed.", + "SKILL.md", + "markdown", + ) + assert any( + finding.rule_id == "PE3" and finding.matched_text.lower() == "access token" + for finding in findings + ), findings + @pytest.mark.parametrize( "instruction", [ diff --git a/uv.lock b/uv.lock index 8e2c47c9..80c5f2f2 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 2 +revision = 3 requires-python = ">=3.12, <3.15" resolution-markers = [ "python_full_version >= '3.14' and sys_platform == 'win32'", @@ -2675,7 +2675,7 @@ wheels = [ [[package]] name = "skillspector" -version = "2.5.3" +version = "2.8.1" source = { editable = "." } dependencies = [ { name = "boto3" },