chore: sync upstream NVIDIA/SkillSpector 2.8.1 (inference usage telemetry) - #13
Merged
Conversation
Sync OSS release snapshot
AnthropicProvider ignored ANTHROPIC_BASE_URL and always constructed ChatAnthropic against the hardcoded https://api.anthropic.com, so proxy and gateway deployments were silently bypassed. This is inconsistent with OpenAIProvider, which already honors OPENAI_BASE_URL. Read ANTHROPIC_BASE_URL in resolve_credentials() and pass it through in create_chat_model(), falling back to api.anthropic.com when unset -- mirroring the OpenAIProvider pattern. Tests: - Add unit coverage for the base_url override and the default-endpoint fallback (resolve_credentials + create_chat_model). - Clear ANTHROPIC_BASE_URL in the provider/llm_utils env-isolation fixtures now that the provider reads it, keeping tests hermetic. - Clear ANTHROPIC_BASE_URL in the live anthropic endpoint test so it validates the default URL, mirroring the OpenAI live test. Signed-off-by: Papastavros Vaggelis <psvaggelis@gmail.com>
Signed-off-by: Keshav Pradeep <32313895+keshprad@users.noreply.github.com>
…e-url fix(anthropic): honor ANTHROPIC_BASE_URL env override
Signed-off-by: Rod Boev <rod.boev@gmail.com>
…NVIDIA#295) Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
fix(provider): isolate Claude CLI settings hooks with --setting-sources=
Signed-off-by: keshavp <32313895+keshprad@users.noreply.github.com>
Re-applies NVIDIA#282, reverted on main by the 2.4.3 OSS snapshot sync (a54947c). resolve_credentials() now reads ANTHROPIC_BASE_URL and create_chat_model() uses base_url or the default endpoint, so proxy/gateway deployments route through the configured host again. Regression coverage for the override and default paths restored. Signed-off-by: Papastavros Vaggelis <psvaggelis@gmail.com>
* chore(release): publish 2.4.4 snapshot Publish the public 2.4.4 package metadata and changelog entry for the internal import of GitHub PR 301. Preserve existing GitHub workflows and public provider tests. Verification: make lint; make format-check; make test-unit; git diff --check origin/main. Signed-off-by: Keshav Pradeep <32313895+keshprad@users.noreply.github.com> * feat(release): publish GitHub releases from labeled PRs Refresh the existing OSS snapshot with the public label-gated release workflow and preserve public changelog source titles. Signed-off-by: Keshav Pradeep <32313895+keshprad@users.noreply.github.com> --------- Signed-off-by: Keshav Pradeep <32313895+keshprad@users.noreply.github.com>
Publish the sanitized 2.5.0 public snapshot with execution ledger and completeness reporting.\n\nThis retains public GitHub workflows, public documentation, and the versioned release notes while excluding internal release tooling, provider implementation, and implementation-planning documents.\n\nVerification: the current internal main pipeline passed lint, unit, integration, Docker smoke, and Sonar. Snapshot integrity and diff checks passed; local tests were not rerun per release guidance. Signed-off-by: Keshav Pradeep <32313895+keshprad@users.noreply.github.com>
…IDIA#305) `arun_batches` bursts up to a hardcoded `max_concurrency=10`. On a rate-limited provider (free tiers with a low RPM) that burst guarantees 429s, and 429'd batches are dropped from the result — silently losing analyzer coverage, including the security-critical `semantic_security_discovery` batch, while the report still renders a full risk_assessment. Add `SKILLSPECTOR_MAX_LLM_CONCURRENCY` so users can serialize the fan-out (set it to 1) and stay under the provider's rate limit. The default stays 10; an explicit `max_concurrency=` argument still wins. Invalid values fall back to the default, values < 1 clamp to 1. Part of NVIDIA#303 (the retry/backoff and partial-pass surfacing are separate). Adds unit tests for the env resolution. Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com> Signed-off-by: keshprad <32313895+keshprad@users.noreply.github.com> Co-authored-by: Mark2Mac <Mark2Mac@users.noreply.github.com> Co-authored-by: keshprad <32313895+keshprad@users.noreply.github.com>
Signed-off-by: keshavp <keshavp@nvidia.com>
Closes NVIDIA#21. Closes NVIDIA#131. The per-file analysis cap (MAX_FILE_BYTES, 1 MB) sits downstream of InputHandler.resolve(), which pulls scan targets from URLs, zips, or git clones with no size budget of its own. As a result, the per-file cap can be defeated upstream: a multi-GB URL is a memory DoS, a zip bomb fills the disk before extraction is gated, and a large clone lands on disk regardless of the per-file analysis budget. PR NVIDIA#19 explicitly deferred this; this PR addresses the deferred work. Adds two ingest budgets enforced at every remote/archive ingest path: - INGEST_MAX_BYTES (100 MiB): caps streamed URL downloads, total uncompressed size of zip archives, and post-clone disk usage of Git repos. - INGEST_MAX_ZIP_MEMBERS (10,000): caps the number of entries in a single zip (defends against the "many tiny files" zip-bomb variant). Per ingest path: - _download_file streams the body in 64 KiB chunks directly to a temp file inside the existing session temp dir, with a running byte counter. The cap check fires before each chunk is written, so the response body is never accumulated in memory. Content-Length is checked up-front when the server provides it (aborts before reading any body bytes); the streamed counter is authoritative if the header is missing or malformed. A breach mid-stream removes the partial file before the exception propagates, so an attacker who ships exactly INGEST_MAX_BYTES + 1 bytes cannot fill the temp dir. - _extract_zip sums ZipInfo.file_size across all members and checks the member count *before* calling extractall, so classic zip bombs (small archive, huge declared uncompressed size) are rejected without materialising any of the bomb on disk. - _clone_git measures the cloned tree's on-disk size after the existing 60s timeout completes and rejects + cleans up if it exceeds the cap. Symlinks are skipped to avoid runaway counts on malicious /dev/zero-style links. All limit breaches raise IngestLimitExceededError (subclass of ValueError so existing callers that catch ValueError keep working) with a clear message including the limit and the observed size. Adds 10 unit tests covering: under-cap success paths for URL/zip/clone; oversized Content-Length rejected before body read; chunked overflow caught by the streamed counter; *the partial download file is removed when a breach fires mid-stream*; *streaming to disk produces a file of the expected size with no intermediate in-memory concatenation*; declared-uncompressed-oversize zip rejected without extraction; member-count zip-bomb rejected; oversize clone rejected and cleaned up. Full suite: 730 passed, 12 skipped. README documents both caps and their relationship to the per-file analysis cap. Signed-off-by: Rohan Isawe <rohan.isawe@smartsheet.com> Co-authored-by: Rohan Isawe <rohan.isawe@smartsheet.com>
…cans (NVIDIA#103) (NVIDIA#232) * fix(analyzer): reduce instructional-prose false positives in static scans (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): preserve direct warning-suppression detection (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): honor quoted and declared benign roles (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): keep adjacent live anti-refusal directives detectable (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): scope benign anti-refusal continuations precisely (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): distinguish declaration headers from live directives (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): treat documentation labels as prose, not examples (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * test(analyzer): cover declaration and fixture prose edges (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): keep live directives from slipping past prose guards (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): keep ambiguous labels from suppressing live directives (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): preserve live directives through the static runner (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): keep block labels and schema prose from masking live directives (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): preserve the remaining AR2 response-suppression directives (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): preserve multiline documentation directives (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): scope anti-refusal suppression to local clauses (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): tighten anti-refusal benign framing (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> * fix(analyzer): tighten anti-refusal suppression framing (NVIDIA#103) Signed-off-by: Rod Boev <rod.boev@gmail.com> --------- Signed-off-by: Rod Boev <rod.boev@gmail.com>
…#238) * Pin actions/checkout usage to avoid dependency confusion attack Signed-off-by: Steven Moy <github@stevenmoy.com> * Pin Docker file dependency to avoid dependency confusion Signed-off-by: Steven Moy <github@stevenmoy.com> --------- Signed-off-by: Steven Moy <github@stevenmoy.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Aamir Akram <aamirakram252003@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
* fix: exclude valid OMS signatures from content analysis OMS bundles necessarily contain long base64-encoded payload, signature, and certificate fields. Generic obfuscated-code checks can misclassify these fields as hidden executable content. Recognize root-level skill.oms.sig files with the minimal OMS v0.3 DSSE/in-toto structure and inventory them as oms_signature components without sending their contents through static or LLM analyzers. Preserve normal scanning for malformed, nested, oversized, or unrecognized signature files. Add a pinned real-world fixture, regression coverage, completeness accounting, and documentation. Recognition is structural only and does not verify signatures, certificates, transparency logs, or signer identity. Signed-off-by: major-security <73253177+major-security@users.noreply.github.com> * fix: accept versioned OMS signature predicates Signed-off-by: Daniel Major <dmajor@nvidia.com> * test: cover OMS exclusions across report formats Signed-off-by: Daniel Major <dmajor@nvidia.com> * fix: preserve semantic ledger evidence for missing files Signed-off-by: Daniel Major <dmajor@nvidia.com> --------- Signed-off-by: major-security <73253177+major-security@users.noreply.github.com> Signed-off-by: Daniel Major <dmajor@nvidia.com>
…al (NVIDIA#321) * fix(static): markdown table and quote syntax is not an execution signal `_is_documentation_context` refuses to treat a line as prose when `_EXECUTION_SIGNAL` matches it, and that pattern includes `[|>]`. In markdown those two characters are structure, not shell metacharacters: `|` delimits table cells and `>` starts a block quote. A governed rule that lands on a table row or a quoted paragraph is therefore never classified as prose, whatever it says. The delimiters are now removed before the execution test, and only the delimiters. Inside a table cell a literal pipe has to be written `\|` (CommonMark), so a documented `cmd \| tee log` keeps its pipe and still counts as an execution signal — which is what makes this safe rather than a widening. Scope, stated plainly: this is a correctness fix, not a precision win. On a corpus of 65 real skill/plugin units (4415 findings, all triaged by hand) only 8 findings of the governed rules were blocked by markdown structure alone, across 4 files. After the change 1 remains, and it has a genuine execution signal on the line. The reason to fix it is that the classification is simply wrong, not that it is frequent. Nothing is added to `_SEMANTIC_STRING_DOC_PRONE_RULES`: the set stays {RA1, TM1, AR2}, and the reasoning that excludes PE3 is untouched. Tests: table row and block quote are classified as prose; a literal escaped pipe in a cell and a real redirection in a quote still are not; plus a unit test that `_strip_markdown_structure` touches delimiters and nothing else. Full suite: 1565 passed, 14 skipped, 6 xfailed. Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com> * fix(static): make the docstring raw so 3.12 stops warning The docstring quotes CommonMark's escaped bar, and \| is not a valid escape sequence: Python 3.12 emits SyntaxWarning on import, which makes a supported-Python test run noisy. The prefix is the whole fix. Guarded package-wide rather than per-file: the new test compiles every shipped module and fails on any SyntaxWarning, so the next one is caught where it is written instead of in a reviewer's terminal. Tests: tests/unit 727 passed, 12 skipped. Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com> --------- Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com> Co-authored-by: Mark2Mac <Mark2Mac@users.noreply.github.com>
…atch (NVIDIA#317) On Windows, run_agent_cli's TemporaryDirectory raised WinError 32 at __exit__ whenever any live process still held the per-invocation temp cwd — and the agent CLI's process tree routinely does: the binary is a .cmd shim whose node children can outlive the direct child, and a killed-but-unreaped child on the timeout path holds its cwd too. A directory that is any live process's working directory (or contains any open handle) cannot be removed on Windows, so cleanup raised *after* the model had already answered, and llm_analyzer_base recorded the batch as llm_batch_failed. Under batch concurrency the teardown window widens, which is why multi-file scans failed while single-file scans passed (NVIDIA#315). Each invocation already has a unique mkdtemp directory, so this was never a cross-worker path collision; it is delete-at-exit racing the process tree's teardown. Fix: mkdtemp + explicit best-effort cleanup. _cleanup_temp_dir retries briefly (10 x 0.2s) to reclaim the directory once the holder exits, then leaks it with a warning instead of raising. A cleanup failure never outranks a successful response. Verified on Windows 10 with a mechanism-faithful fake CLI (a .cmd shim that detaches a grandchild holding the temp cwd): 8 concurrent batches fail 8/8 with the exact WinError 32 signature from NVIDIA#315 on main, and pass 8/8 with this change. Controlled experiments confirm both hold modes (another process's cwd; an open file handle inside the dir) block rmtree with WinError 32. Tests: 5 new cases including a Windows-only real-handle hold and a regression test asserting run_agent_cli returns the response when rmtree keeps failing. tests/unit/test_agent_cli.py 87/87 on Windows. Closes NVIDIA#315 Signed-off-by: ppcvote <risky9763@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…fy (NVIDIA#318) (NVIDIA#319) * fix(supply-chain): treat only == / <= as version pins in requirements.txt `_extract_packages_from_requirements` kept the captured version for any operator, so a floor like `pillow>=10.0.0` was recorded as the exact release `10.0.0` and the OSV/CVE lookup attributed that version's vulnerabilities to an unpinned dependency — a false CRITICAL on a requirements file that pins nothing. Only `==` and `<=` bound the dependency to a concrete, CVE-checkable release; `>=`, `>`, `!=`, `~=` are floors/ranges. This mirrors the guard already present in `_extract_packages_from_setup_py` (`m.group(2) in ("==", "<=")`), so the two extractors now agree. Added a regression test asserting `>=`, `~=`, `!=` and bare names yield version=None while `==` / `<=` keep the version. Fixes NVIDIA#294 Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com> * fix(supply-chain): only exact pins resolve to a version, ranges do not Addresses the review on NVIDIA#302: the previous guard still admitted non-exact constraints. `<=8.1.0` matches every earlier release and `==1.*` is a wildcard, so both were handed to the vulnerability lookup as a version the dependency may never install. A vulnerability lookup answers "is THIS release affected?", which is only meaningful when the manifest admits exactly one release. That predicate is now explicit and shared instead of being re-derived at each call site: - `_pinned_version` (PEP 440): only `==` with a fully concrete version. Floors, caps, exclusions, compatible releases and wildcard equality yield None. - `_pinned_npm_version` (semver): only a bare `x.y.z`. npm defaults to caret ranges, so `"^1.8.3"` was being stripped into the concrete release `1.8.3`. Applied to all three extractors — requirements.txt, pyproject.toml and package.json — because the objection in the review holds verbatim for the two that were not touched by the original patch. Note for the maintainer: dropping these specifiers moves more dependencies to version=None, which NVIDIA#318 shows is currently reported as CRITICAL carrying the package's worst-ever advisory. The two fixes are complementary; happy to send the severity side as a separate PR. Regressions cover both cases named in the review (`<=` and `==1.*`) plus the npm caret/tilde/wildcard/range forms. Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com> * fix(supply-chain): SC4 must not claim a vulnerability it did not verify When a manifest admits a range, no version is resolved and OSV is queried by name alone. The advisories that come back are the package's history, not a match against the release that will be installed: the worst of them may predate every version the range admits. Using that as the finding's severity turns 'setuptools>=61' into a CRITICAL 'Known Vulnerable Dependency'. Scanning 65 skill/plugin units, every SC4 finding in the corpus came from this or from a range being read as a pin (NVIDIA#294/NVIDIA#302). Not one manifest pinned a vulnerable release. The lack of pinning is already reported by SC1, so what is left for SC4 to say is 'could not verify', and it must not outrank a real version match: severity capped at LOW, confidence 0.4, and wording that states the limit instead of implying a match. Version-matched findings are unchanged. Closes NVIDIA#318 Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com> * fix(supply-chain): preserve full PEP 440 pins Signed-off-by: keshavp <32313895+keshprad@users.noreply.github.com> * fix(supply-chain): parse pip requirement options Signed-off-by: keshavp <32313895+keshprad@users.noreply.github.com> * style: format supply chain tests Signed-off-by: keshavp <32313895+keshprad@users.noreply.github.com> --------- Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com> Signed-off-by: keshavp <32313895+keshprad@users.noreply.github.com> Co-authored-by: Mark2Mac <Mark2Mac@users.noreply.github.com> Co-authored-by: keshavp <32313895+keshprad@users.noreply.github.com>
`_is_layout_only_span()` is what keeps MP2 from reporting alignment as context
stuffing, and it has no test. The only layout case covered today is `"=" * 80`,
which never reaches the helper — the older single-character guard skips it first.
Measured on a corpus of 4406 files from 65 skill/plugin units, comparing the
analyzer before the helper landed with `main` today:
MP2 findings 279 -> 4
The four survivors come from MP2's prose patterns ("exceed context window"), not
from the repetition pattern. So the helper is carrying 275 of 279 findings, with
nothing pinning its behaviour.
The three cases added here are each reported when the helper is stubbed out, which
is what makes them regressions rather than restatements of the guard above them:
a dash rule, a run of padded columns, and a box edge with padding. The fourth test
locks the other side, `_MAX_LAYOUT_ONLY_SPAN`: past that width layout stops being a
plausible explanation and the run is reported again.
No source change.
Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com>
Co-authored-by: Mark2Mac <Mark2Mac@users.noreply.github.com>
The README has no mention of the Verified Skills pipeline, the skills catalog, or the hosted documentation, so a reader arriving at this repo has no path to any of them. - Overview: one line placing SkillSpector in the pipeline, linking the docs home and the skills catalog. - Documentation: lead with the hosted scanning guide, ahead of the repo-local developer docs, since that is what most readers want. Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
Signed-off-by: Keshav Prasad <32313895+keshprad@users.noreply.github.com>
* fix(output-handling): ignore RegExp exec parsing Signed-off-by: Christopher Kevin <christopherk@nvidia.com> * fix(output-handling): guard RegExp exec mutations Signed-off-by: Christopher Kevin <christopherk@nvidia.com> * fix(output-handling): avoid unverified mutation inference Signed-off-by: Christopher Kevin <christopherk@nvidia.com> * test(output-handling): cover regexp class syntax Signed-off-by: Christopher Kevin <christopherk@nvidia.com> * fix(output-handling): fail closed across line comments Signed-off-by: Christopher Kevin <christopherk@nvidia.com> * fix(output-handling): handle legacy HTML comments Signed-off-by: Christopher Kevin <christopherk@nvidia.com> --------- Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
…on (NVIDIA#332) * fix(analyzer): detect full environment reads via AST Signed-off-by: Mohit Gupta <mohgupta@nvidia.com> * refactor(analyzers): share Python AST parsing Signed-off-by: Mohit Gupta <mohgupta@nvidia.com> * fix(analyzer): detect keyword environment secret reads Signed-off-by: keshavp <32313895+keshprad@users.noreply.github.com> --------- Signed-off-by: Mohit Gupta <mohgupta@nvidia.com> Signed-off-by: keshavp <32313895+keshprad@users.noreply.github.com> Co-authored-by: keshavp <32313895+keshprad@users.noreply.github.com>
Signed-off-by: keshprad <32313895+keshprad@users.noreply.github.com>
Sanitized from internal release 589281bf188f28eb59d7db8fe10858612b5f3d1f after the repository OSS scrubber and full unit-test gate.
smoy
approved these changes
Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NVIDIA/SkillSpectorrelease 2.8.1 (0a1546b) into main — the oldest release that ships LLM token-usage telemetry (metadata.inference_usagein JSON reports, seedocs/INFERENCE_USAGE.md).src/testsremain byte-identical to the upstream snapshot apart from the existingsrc/skillspector/exaforce/patch module.Details
No structured plan — upstream snapshot sync, no fork-authored code changes.
Tests: full suite
uv run pytest -qon this branch: 1917 passed, 13 skipped, 4 failed. The 4 failures are pre-existing on main (verified by running the same tests in a clean worktree at main) and are by design: the fork'sexaforceschema patches prune LLM prose fields (explanation,remediation,intent,impact,overall_assessment) that these upstream tests assert (test_to_finding,test_model_dump,test_intent_validationintests/nodes/test_llm_analyzer_base.py;test_malicious_skill_findings_preserve_metadataintests/nodes/test_semantic_quality_policy.py). ThePatchDriftErrorguard did not fire — fork patches apply cleanly against 2.8.1.Note:
pre-commit(ruff, config at upstream parity) flags 21 issues in upstream's owncontrib/batch_scan/files. Left untouched deliberately — reformatting upstream-tracked files would break parity and cause conflicts on the next sync.