diff --git a/README.md b/README.md index 0da5bdd..5e773a9 100644 --- a/README.md +++ b/README.md @@ -181,6 +181,8 @@ inference gateways. | `anthropic` | `ANTHROPIC_API_KEY` | api.anthropic.com | `claude-opus-4-6` | | `anthropic_proxy` | `ANTHROPIC_PROXY_API_KEY` + `ANTHROPIC_PROXY_ENDPOINT_URL` | Any Vertex-style raw-predict proxy | `claude-sonnet-4-6` | | `nv_build` | `NVIDIA_INFERENCE_KEY` | build.nvidia.com | `deepseek-ai/deepseek-v4-flash` | +| `claude_cli` | _(none — uses local CLI auth)_ | local `claude` binary | `claude-sonnet-4-6` | +| `codex_cli` | _(none — uses local CLI auth)_ | local `codex` binary | `o4-mini` | ```bash # Stock OpenAI @@ -205,6 +207,16 @@ export SKILLSPECTOR_PROVIDER=nv_build export NVIDIA_INFERENCE_KEY=nvapi-... skillspector scan ./my-skill/ +# Local Claude CLI — no API key; uses your existing `claude auth login` session +# Requires: claude CLI installed and authenticated (claude auth login) +export SKILLSPECTOR_PROVIDER=claude_cli +skillspector scan ./my-skill/ + +# Local Codex CLI — no API key; uses your existing `codex login` session +# Requires: codex CLI installed and authenticated +export SKILLSPECTOR_PROVIDER=codex_cli +skillspector scan ./my-skill/ + # Local Ollama or any OpenAI-compatible endpoint export SKILLSPECTOR_PROVIDER=openai export OPENAI_API_KEY=ollama @@ -478,7 +490,7 @@ Issues (2) | Variable | Description | Required | |----------|-------------|----------| -| `SKILLSPECTOR_PROVIDER` | Active LLM provider: `openai`, `anthropic`, or `nv_build`. Each provider has its own bundled `model_registry.yaml` and default model (see the LLM Analysis table above). Defaults to `nv_build`. | Optional | +| `SKILLSPECTOR_PROVIDER` | Active LLM provider: `openai`, `anthropic`, `nv_build`, `claude_cli`, or `codex_cli`. Each provider has its own bundled `model_registry.yaml` and default model (see the LLM Analysis table above). Defaults to `nv_build`. | Optional | | `NVIDIA_INFERENCE_KEY` | Credential for the `nv_build` provider (build.nvidia.com). | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=nv_build` | | `OPENAI_API_KEY` | Credential for the OpenAI provider (`SKILLSPECTOR_PROVIDER=openai`). Also serves as the tier-2 fallback in the credential waterfall when the active provider returns no credentials. | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=openai` | | `OPENAI_BASE_URL` | Override the OpenAI endpoint (e.g. point at Ollama). | Optional | @@ -490,6 +502,8 @@ Issues (2) | `SKILLSPECTOR_MODEL_REGISTRY` | Override the bundled per-provider YAML registry (`src/skillspector/providers//model_registry.yaml`) with a custom path. | Optional | | `SKILLSPECTOR_LOG_LEVEL` | Log level: `DEBUG`, `INFO`, `WARNING`, `ERROR` (default: `WARNING`). | Optional | +> **CLI providers** (`claude_cli`, `codex_cli`): No API key is needed. Authentication is managed entirely by the agent CLI's own login session (`claude auth login` / `codex login`). SkillSpector never reads or forwards API keys when these providers are active. The subprocess is run in a hardened sandbox: tools disabled, no MCP, read-only sandbox mode (codex), and untrusted skill content is delivered only via stdin. + ### CLI Options ```bash diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index a9f31f0..f910c32 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -265,12 +265,14 @@ Copy [.env.example](../.env.example) to `.env` in the project root and set value | Variable | Description | Example | |----------|-------------|---------| -| `SKILLSPECTOR_PROVIDER` | Active LLM provider: `openai` \| `anthropic` \| `nv_build`. Defaults to `nv_build`. | `openai` | +| `SKILLSPECTOR_PROVIDER` | Active LLM provider: `openai` \| `anthropic` \| `nv_build` \| `claude_cli` \| `codex_cli`. Defaults to `nv_build`. | `claude_cli` | | `NVIDIA_INFERENCE_KEY` | Credential for `nv_build`. | `nvapi-...` | | `OPENAI_API_KEY` | Credential for `SKILLSPECTOR_PROVIDER=openai`. Also tier-2 fallback for non-OpenAI providers. | `sk-...` | | `OPENAI_BASE_URL` | Override the OpenAI endpoint (e.g. point at Ollama). | `http://localhost:11434/v1` | | `ANTHROPIC_API_KEY` | Credential for `SKILLSPECTOR_PROVIDER=anthropic`. | `sk-ant-...` | -| `SKILLSPECTOR_MODEL` | Override the active provider's bundled default model (see [README.md](../README.md) for per-provider defaults). | `gpt-5.2` | +| `SKILLSPECTOR_MODEL` | Override the active provider's bundled default model (see [README.md](../README.md) for per-provider defaults). For `claude_cli`, this is passed as `--model` to the `claude` binary. | `gpt-5.2` | + +> **CLI providers** (`claude_cli`, `codex_cli`): no credential env var is needed. Authentication is managed by the agent CLI's own session (`claude auth login` / `codex login`). The subprocess is heavily sandboxed — see [providers/_agent_cli.py](../src/skillspector/providers/_agent_cli.py). ### Live provider tests @@ -291,8 +293,18 @@ Base URL env vars are not needed for live provider tests; the tests intentionall - **`get_max_input_tokens(model)`** — input budget per LLM request (75% of resolved context window). - **`get_max_output_tokens(model)`** — output budget per LLM request (min of 25% context, registry's `max_output_tokens` cap if set). - Batch budget overhead is computed per-prompt via `estimate_tokens(base_prompt)` rather than a fixed constant. -- **Providers** ([providers/](../src/skillspector/providers/)): pluggable credential + token-budget resolvers. Each provider is a subpackage with its own `provider.py` and bundled `model_registry.yaml`; [registry.py](../src/skillspector/providers/registry.py) exposes `lookup_context_length` / `lookup_max_output_tokens` utilities the providers call directly. The active provider is chosen by `SKILLSPECTOR_PROVIDER` (default: `nv_build`) — see [providers/`__init__`.py](../src/skillspector/providers/__init__.py): `nv_build/` (build.nvidia.com), `openai/`, or `anthropic/`. -- **LLM calls** ([llm_utils.py](../src/skillspector/llm_utils.py)): **`get_chat_model()`** and **`chat_completion()`** resolve credentials in two tiers — active NVIDIA provider (`NVIDIA_INFERENCE_KEY` → endpoint) → standard `OPENAI_API_KEY` / `OPENAI_BASE_URL` — against any OpenAI-compatible endpoint. `max_tokens` is auto-bound to `get_max_output_tokens(model)` from `model_info`. +- **Providers** ([providers/](../src/skillspector/providers/)): pluggable credential + token-budget resolvers. Each provider is a subpackage with its own `provider.py` and bundled `model_registry.yaml`; [registry.py](../src/skillspector/providers/registry.py) exposes `lookup_context_length` / `lookup_max_output_tokens` utilities the providers call directly. The active provider is chosen by `SKILLSPECTOR_PROVIDER` (default: `nv_build`): + - `nv_build/` — build.nvidia.com (HTTP, `NVIDIA_INFERENCE_KEY`) + - `openai/` — api.openai.com or any OpenAI-compatible URL (`OPENAI_API_KEY`) + - `anthropic/` — api.anthropic.com (`ANTHROPIC_API_KEY`) + - `claude_cli/` — **local `claude` binary; no API key**. Uses the CLI's own auth session (`claude auth login`). Set `SKILLSPECTOR_PROVIDER=claude_cli`. + - `codex_cli/` — **local `codex` binary; no API key**. Uses the CLI's own auth session (`codex login`). Set `SKILLSPECTOR_PROVIDER=codex_cli`. + + CLI providers (`claude_cli`, `codex_cli`) implement the optional `AgentCLICapable` interface (`is_available()` + `complete()`) defined in [providers/base.py](../src/skillspector/providers/base.py). `has_cli_capability(provider)` detects this at runtime. All subprocess calls go through the hardened helper [providers/_agent_cli.py](../src/skillspector/providers/_agent_cli.py) which enforces: no shell (`shell=False`), untrusted content via stdin only, capability stripping (tools disabled / sandboxed), environment scrubbing (no API keys forwarded), per-call timeout, and fail-closed error handling. + +- **LLM calls** ([llm_utils.py](../src/skillspector/llm_utils.py)): **`get_chat_model()`** and **`chat_completion()`** dispatch based on the active provider: + - **HTTP providers**: resolve credentials in two tiers — active provider (`NVIDIA_INFERENCE_KEY` / `ANTHROPIC_API_KEY` / `OPENAI_API_KEY` → endpoint) — against any OpenAI-compatible endpoint. `max_tokens` is auto-bound to `get_max_output_tokens(model)` from `model_info`. + - **CLI providers** (`claude_cli`, `codex_cli`): `get_chat_model()` returns an `AgentCLIChatModel` adapter backed by `provider.complete()`, so the analyzers' `.invoke()` / `.with_structured_output(schema).invoke()` calls work with no API key (structured output is produced by prompting for JSON, then Pydantic-validating). `chat_completion()` routes through `get_chat_model()` as well. `is_llm_available()` calls `provider.is_available()` instead of credential resolution. - **LLM analyzer base** ([llm_analyzer_base.py](../src/skillspector/nodes/llm_analyzer_base.py)): `LLMAnalyzerBase` provides per-file/per-chunk batching, token-budget-aware chunking, and a run loop for all LLM-based analyzers. `LLMMetaAnalyzer` extends it for filter/enrich (meta_analyzer node). Future semantic analyzers extend `LLMAnalyzerBase` for discovery mode. --- diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index d1c5104..468e26b 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -13,13 +13,17 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Shared LLM utilities. +"""Shared LLM utilities (OpenAI-compatible chat models + agent CLI transports). Credentials are resolved in this order: - 1. The active SkillSpector provider (see :mod:`skillspector.providers`) — - reads its own credential env var and supplies the matching client. + 1. The active provider (see :mod:`skillspector.providers`): + - CLI providers (``claude_cli``, ``codex_cli``, ``gemini_cli``): use + ``is_available()`` and ``complete()`` — no API key needed. + - HTTP providers (``anthropic``, ``openai``, ``nv_build``): read their + respective credential env vars and supply a base URL. 2. ``OPENAI_API_KEY`` / ``OPENAI_BASE_URL`` (the langchain-openai - defaults). + defaults) — only consulted for HTTP providers when the provider's + own credential env var is unset. There is no SkillSpector-specific credential env var: setting ``NVIDIA_INFERENCE_KEY`` configures whichever NVIDIA endpoint the @@ -30,13 +34,18 @@ from __future__ import annotations +import asyncio +import json +from typing import NoReturn + from langchain_core.language_models.chat_models import BaseChatModel -from langchain_core.messages import BaseMessage from skillspector.model_info import get_max_input_tokens, get_max_output_tokens from skillspector.providers import ( create_chat_model, + get_active_provider, get_metadata_provider, + has_cli_capability, raise_no_llm_api_key_configured, resolve_chat_model_credentials, resolve_provider_credentials, @@ -47,8 +56,9 @@ def _resolve_llm_credentials() -> tuple[str, str | None]: """Return ``(api_key, base_url)`` resolved from the environment. - Tries the active NVIDIA provider first; falls back to ``OPENAI_API_KEY`` - / ``OPENAI_BASE_URL`` when the provider is not configured. + Tries the active SkillSpector provider first; falls back to + ``OPENAI_API_KEY`` / ``OPENAI_BASE_URL`` when the provider is not + configured. Raises: ValueError: when no API key can be resolved from any source. @@ -72,7 +82,15 @@ def _resolve_default_chat_model() -> str: def is_llm_available() -> tuple[bool, str | None]: - """Return ``(available, error_message)`` describing LLM credential status.""" + """Return ``(available, error_message)`` describing LLM availability. + + For CLI providers (``claude_cli``, ``codex_cli``, ``gemini_cli``) the check + delegates to the provider's ``is_available()`` method (binary on PATH + + auth). For HTTP providers, it falls back to credential resolution. + """ + provider = get_active_provider() + if has_cli_capability(provider): + return provider.is_available() # type: ignore[attr-defined] try: _resolve_llm_credentials() except ValueError as exc: @@ -85,12 +103,157 @@ def fetch_model_token_limits(model_label: str) -> tuple[int, int]: return get_max_input_tokens(model_label), get_max_output_tokens(model_label) -def get_chat_model(model: str | None = None) -> BaseChatModel: - """Return the active provider's native LangChain chat model. +# --------------------------------------------------------------------------- +# Agent CLI chat-model adapter +# --------------------------------------------------------------------------- +# +# The LLM analyzers (meta_analyzer, semantic_*) obtain a model from +# ``get_chat_model()`` and call ``.invoke()`` / ``.with_structured_output( +# schema).invoke()`` on it (see ``llm_analyzer_base``) — they never go through +# ``chat_completion``. To support CLI providers there, ``get_chat_model`` +# returns this minimal adapter, which mimics the slice of the ``ChatOpenAI`` +# interface the analyzers rely on, backed by the provider's ``complete()`` +# subprocess transport. + + +class _AgentCLIMessage: + """Minimal stand-in for a LangChain message: exposes ``.content``.""" + + def __init__(self, content: str) -> None: + self.content = content + + +def _extract_json_object(raw: str) -> dict: + """Extract a single JSON object from a CLI model's text response. + + Tolerates markdown code fences and surrounding prose. Raises ``ValueError`` + (fail-closed) when no JSON object can be parsed. + """ + text = raw.strip() + if text.startswith("```"): + # Drop the opening fence line (``` or ```json) and any closing fence. + text = text.split("\n", 1)[1] if "\n" in text else "" + fence = text.rfind("```") + if fence != -1: + text = text[:fence] + text = text.strip() + try: + obj = json.loads(text) + if isinstance(obj, dict): + return obj + except json.JSONDecodeError: + pass + start, end = text.find("{"), text.rfind("}") + if start != -1 and end > start: + try: + obj = json.loads(text[start : end + 1]) + if isinstance(obj, dict): + return obj + except json.JSONDecodeError: + pass + raise ValueError(f"could not extract a JSON object from CLI response: {raw[:200]!r}") + + +class _StructuredAgentCLIModel: + """Mimics ``ChatOpenAI.with_structured_output(schema)`` for a CLI provider. + + ``invoke`` augments the prompt with the schema, calls the provider's + ``complete()``, then parses and validates the response into *schema*. + """ + + def __init__(self, provider: object, model: str, max_output_tokens: int, schema: type) -> None: + self._provider = provider + self._model = model + self._max_output_tokens = max_output_tokens + self._schema = schema + + def _augment(self, prompt: str) -> str: + schema_json = json.dumps(self._schema.model_json_schema(), indent=2) + return ( + f"{prompt}\n\n" + "Respond with ONLY a single JSON object conforming to the JSON Schema " + "below. Do not wrap it in markdown code fences and do not add any prose " + 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] + self._augment(prompt), + model=self._model, + max_output_tokens=self._max_output_tokens, + ) + return self._schema.model_validate(_extract_json_object(raw)) + + async def ainvoke(self, prompt: str) -> object: + return await asyncio.to_thread(self.invoke, prompt) + + +class AgentCLIChatModel: + """Minimal ``ChatOpenAI``-compatible adapter backed by a CLI provider. + + Implements only the surface the analyzers use: ``invoke`` (returns an + object with ``.content``), ``ainvoke``, and ``with_structured_output``. + The rest of the ``BaseChatModel`` surface (``batch``, ``stream``, + callbacks) is intentionally unsupported; the stubs below make that boundary + explicit so a future analyzer reaching for it fails loudly with a clear + message rather than a confusing ``AttributeError``. + """ + + def __init__(self, provider: object, model: str, max_output_tokens: int) -> None: + self._provider = provider + self._model = model + self._max_output_tokens = max_output_tokens + + def batch(self, *args: object, **kwargs: object) -> NoReturn: + raise NotImplementedError( + "AgentCLIChatModel supports only invoke/ainvoke/with_structured_output; " + "batch() is not available for CLI providers." + ) + + def stream(self, *args: object, **kwargs: object) -> NoReturn: + raise NotImplementedError( + "AgentCLIChatModel supports only invoke/ainvoke/with_structured_output; " + "stream() is not available for CLI providers." + ) + + def invoke(self, prompt: str) -> _AgentCLIMessage: + text = self._provider.complete( # type: ignore[attr-defined] + prompt, + model=self._model, + max_output_tokens=self._max_output_tokens, + ) + return _AgentCLIMessage(text) + + async def ainvoke(self, prompt: str) -> _AgentCLIMessage: + return await asyncio.to_thread(self.invoke, prompt) + + def with_structured_output(self, schema: type) -> _StructuredAgentCLIModel: + return _StructuredAgentCLIModel( + self._provider, self._model, self._max_output_tokens, schema + ) + + +def get_chat_model(model: str | None = None) -> BaseChatModel | AgentCLIChatModel: + """Return a chat model for the active provider. + + For CLI providers (``claude_cli``, ``codex_cli``, ``gemini_cli``) this + returns an :class:`AgentCLIChatModel` adapter backed by the provider's + ``complete()`` subprocess transport — so the LLM analyzers (which use + ``.invoke()`` and ``.with_structured_output()``) work with no API key. + + For HTTP providers it delegates to + :func:`skillspector.providers.create_chat_model`, which uses the + provider's own native client (e.g. ``ChatAnthropic`` for Anthropic) with + an ``OPENAI_API_KEY`` / ``ChatOpenAI`` fallback. Raises: - ValueError: when no API key is configured (see ``is_llm_available``). + ValueError: when an HTTP provider has no API key configured. """ + 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)) + model = model or _resolve_default_chat_model() return create_chat_model( model=model, @@ -100,9 +263,16 @@ def get_chat_model(model: str | None = None) -> BaseChatModel: def chat_completion(prompt: str, *, model: str | None = None) -> str: - """Request a single chat completion and return the assistant text.""" - llm = get_chat_model(model=model) - response = llm.invoke(prompt) - if not isinstance(response, BaseMessage): - raise TypeError(f"Expected BaseMessage from chat model, got {type(response).__name__}") - return str(response.text) + """Request a single chat completion and return the assistant content. + + Routes through :func:`get_chat_model`, which dispatches to the CLI adapter + for CLI providers and to the provider's native chat model for HTTP providers. + + Uses ``.text`` when available (real LangChain ``BaseMessage`` objects, + 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) + 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 45d13dc..0974a63 100644 --- a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py +++ b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py @@ -25,7 +25,12 @@ from skillspector.llm_utils import chat_completion from skillspector.models import Finding -from skillspector.state import AnalyzerNodeResponse, SkillspectorState +from skillspector.state import ( + AnalyzerNodeResponse, + LLMCallRecord, + SkillspectorState, + llm_call_record, +) ANALYZER_ID = "mcp_tool_poisoning" logger = logging.getLogger(__name__) @@ -677,13 +682,20 @@ def _check_tp3(params: list[dict]) -> list[Finding]: ) -def _check_tp4(state: SkillspectorState) -> list[Finding]: - """TP4: LLM-based description-behavior mismatch detection.""" +def _check_tp4(state: SkillspectorState) -> tuple[list[Finding], LLMCallRecord | None]: + """TP4: LLM-based description-behavior mismatch detection. + + Returns ``(findings, record)`` 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`. + """ + attempted = False try: manifest: dict = state.get("manifest") or {} description = manifest.get("description") if not description or not isinstance(description, str) or not description.strip(): - return [] + return [], None triggers = manifest.get("triggers") or [] permissions = manifest.get("permissions") @@ -705,7 +717,7 @@ def _check_tp4(state: SkillspectorState) -> list[Finding]: code_parts.append(f"### {path} ({file_type})\n{content}") if not code_parts: - return [] + return [], None code_contents = "\n\n".join(code_parts) @@ -749,6 +761,7 @@ def _check_tp4(state: SkillspectorState) -> list[Finding]: "explanation": "why this is or is not a mismatch" }}""" + attempted = True response = chat_completion(prompt, model=model) # Parse JSON — handle optional ```json code blocks @@ -763,13 +776,14 @@ def _check_tp4(state: SkillspectorState) -> list[Finding]: json_text = json_text.rstrip()[:-3].rstrip() result = json.loads(json_text) + ok_record = llm_call_record(ANALYZER_ID, ok=True) if not result.get("is_mismatch"): - return [] + return [], ok_record confidence = float(result.get("confidence", 0.0)) if confidence < 0.5: - return [] + return [], ok_record severity = "HIGH" if confidence >= 0.7 else "MEDIUM" @@ -797,11 +811,15 @@ def _check_tp4(state: SkillspectorState) -> list[Finding]: "or remove undeclared functionality from the implementation." ), ) - ] + ], ok_record - except Exception: + except Exception as exc: logger.warning("%s: TP4 LLM check failed, skipping", ANALYZER_ID, exc_info=True) - return [] + # 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)) + return [], None # --------------------------------------------------------------------------- @@ -839,8 +857,15 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: # match every other LLM-using node (semantic_*, meta_analyzer); the CLI # always sets this explicitly, so the default only affects programmatic # callers that omit the key. + tp4_record: LLMCallRecord | None = None if state.get("use_llm", True): - findings.extend(_check_tp4(state)) + tp4_findings, tp4_record = _check_tp4(state) + findings.extend(tp4_findings) logger.info("%s: %d findings", ANALYZER_ID, len(findings)) - return {"findings": findings} + result: AnalyzerNodeResponse = {"findings": findings} + # Emit LLM telemetry only when TP4 actually attempted a call, so the report's + # degradation detector counts this node consistently with the semantic ones. + if tp4_record is not None: + result["llm_call_log"] = [tp4_record] + return result diff --git a/src/skillspector/nodes/analyzers/semantic_developer_intent.py b/src/skillspector/nodes/analyzers/semantic_developer_intent.py index a3a54be..92a4769 100644 --- a/src/skillspector/nodes/analyzers/semantic_developer_intent.py +++ b/src/skillspector/nodes/analyzers/semantic_developer_intent.py @@ -27,7 +27,7 @@ from skillspector.constants import _SKILLSPECTOR_DEFAULT_MODEL, MODEL_CONFIG from skillspector.llm_analyzer_base import LLMAnalyzerBase from skillspector.logging_config import get_logger -from skillspector.state import AnalyzerNodeResponse, SkillspectorState +from skillspector.state import AnalyzerNodeResponse, SkillspectorState, llm_call_record ANALYZER_ID = "semantic_developer_intent" logger = get_logger(__name__) @@ -179,9 +179,12 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: results = asyncio.run(analyzer.arun_batches(batches)) findings = analyzer.collect_findings(results) logger.info("%s: %d findings", ANALYZER_ID, len(findings)) - return {"findings": findings} + return {"findings": findings, "llm_call_log": [llm_call_record(ANALYZER_ID, ok=True)]} except ValueError: raise except Exception as exc: logger.warning("%s failed: %s", ANALYZER_ID, exc) - return {"findings": []} + return { + "findings": [], + "llm_call_log": [llm_call_record(ANALYZER_ID, ok=False, error=str(exc))], + } diff --git a/src/skillspector/nodes/analyzers/semantic_quality_policy.py b/src/skillspector/nodes/analyzers/semantic_quality_policy.py index 3140334..40b553e 100644 --- a/src/skillspector/nodes/analyzers/semantic_quality_policy.py +++ b/src/skillspector/nodes/analyzers/semantic_quality_policy.py @@ -27,7 +27,7 @@ from skillspector.constants import _SKILLSPECTOR_DEFAULT_MODEL from skillspector.llm_analyzer_base import LLMAnalyzerBase from skillspector.logging_config import get_logger -from skillspector.state import AnalyzerNodeResponse, SkillspectorState +from skillspector.state import AnalyzerNodeResponse, SkillspectorState, llm_call_record ANALYZER_ID = "semantic_quality_policy" logger = get_logger(__name__) @@ -148,9 +148,12 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: results = asyncio.run(analyzer.arun_batches(batches)) findings = analyzer.collect_findings(results) logger.info("%s: %d findings", ANALYZER_ID, len(findings)) - return {"findings": findings} + return {"findings": findings, "llm_call_log": [llm_call_record(ANALYZER_ID, ok=True)]} except ValueError: raise except Exception as exc: logger.warning("%s failed: %s", ANALYZER_ID, exc) - return {"findings": []} + return { + "findings": [], + "llm_call_log": [llm_call_record(ANALYZER_ID, ok=False, error=str(exc))], + } diff --git a/src/skillspector/nodes/analyzers/semantic_security_discovery.py b/src/skillspector/nodes/analyzers/semantic_security_discovery.py index 62ef4e9..72a0dde 100644 --- a/src/skillspector/nodes/analyzers/semantic_security_discovery.py +++ b/src/skillspector/nodes/analyzers/semantic_security_discovery.py @@ -22,7 +22,7 @@ from skillspector.constants import _SKILLSPECTOR_DEFAULT_MODEL from skillspector.llm_analyzer_base import LLMAnalyzerBase from skillspector.logging_config import get_logger -from skillspector.state import AnalyzerNodeResponse, SkillspectorState +from skillspector.state import AnalyzerNodeResponse, SkillspectorState, llm_call_record ANALYZER_ID = "semantic_security_discovery" logger = get_logger(__name__) @@ -90,13 +90,21 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: results = analyzer.run_batches(batches) findings = analyzer.collect_findings(results) logger.info("%s: %d findings", ANALYZER_ID, len(findings)) - return {"findings": findings} + return {"findings": findings, "llm_call_log": [llm_call_record(ANALYZER_ID, ok=True)]} except ValidationError as exc: # Malformed LLM response — degrade gracefully rather than crashing the graph logger.warning("%s: LLM returned malformed response: %s", ANALYZER_ID, exc) - return {"findings": []} + return { + "findings": [], + "llm_call_log": [ + llm_call_record(ANALYZER_ID, ok=False, error=f"malformed LLM response: {exc}") + ], + } except ValueError: raise except Exception as exc: logger.warning("%s failed: %s", ANALYZER_ID, exc) - return {"findings": []} + return { + "findings": [], + "llm_call_log": [llm_call_record(ANALYZER_ID, ok=False, error=str(exc))], + } diff --git a/src/skillspector/nodes/meta_analyzer.py b/src/skillspector/nodes/meta_analyzer.py index e910bc0..bd044e4 100644 --- a/src/skillspector/nodes/meta_analyzer.py +++ b/src/skillspector/nodes/meta_analyzer.py @@ -39,7 +39,7 @@ get_explanation, get_remediation, ) -from skillspector.state import MetaAnalyzerResponse, SkillspectorState +from skillspector.state import MetaAnalyzerResponse, SkillspectorState, llm_call_record logger = get_logger(__name__) @@ -521,9 +521,11 @@ def meta_analyzer(state: SkillspectorState) -> MetaAnalyzerResponse: metadata_text = _format_metadata(manifest) files_with_findings = sorted({f.file for f in findings}) - analyzer = LLMMetaAnalyzer(model=model) - try: + # Construct inside the try so a chat-model construction failure is caught + # and recorded as a degraded LLM call (consistent with the semantic + # analyzers) rather than crashing the whole graph. + analyzer = LLMMetaAnalyzer(model=model) batches = analyzer.get_batches(files_with_findings, file_cache, findings) logger.debug( "Meta-analyzer: %d files -> %d batches (model=%s)", @@ -564,9 +566,15 @@ def meta_analyzer(state: SkillspectorState) -> MetaAnalyzerResponse: len(findings), len(filtered), ) - return {"filtered_findings": filtered} + return { + "filtered_findings": filtered, + "llm_call_log": [llm_call_record("meta_analyzer", ok=True)], + } except ValueError: raise except Exception as e: logger.warning("LLM call failed, passing all findings through (fail-closed): %s", e) - return {"filtered_findings": _passthrough_with_defaults(findings)} + return { + "filtered_findings": _passthrough_with_defaults(findings), + "llm_call_log": [llm_call_record("meta_analyzer", ok=False, error=str(e))], + } diff --git a/src/skillspector/nodes/report.py b/src/skillspector/nodes/report.py index 48e15d3..e61e17e 100644 --- a/src/skillspector/nodes/report.py +++ b/src/skillspector/nodes/report.py @@ -39,9 +39,11 @@ SARIF_SCHEMA_URI, SarifArtifactLocation, SarifDriver, + SarifInvocation, SarifLocation, SarifLog, SarifMessage, + SarifNotification, SarifPhysicalLocation, SarifRegion, SarifReportingDescriptor, @@ -138,11 +140,19 @@ def _compute_risk_score( def _build_sarif( findings: list[Finding], suppressed: list[SuppressedFinding] | None = None, + degraded_notice: str | None = None, ) -> dict[str, object]: """Build SARIF 2.1.0 log from findings. Filters out empty/malformed findings (missing rule_id or message) and builds the required tool.driver.rules[] array from referenced rule IDs. + + When *degraded_notice* is set (the LLM stage was requested but every call + failed), a single ``invocation`` is added carrying the notice as a + warning-level ``toolExecutionNotifications`` entry — the standard SARIF + place for execution-time conditions — so the default output format also + surfaces the degradation. ``executionSuccessful`` stays True: the scan + completed and produced results; only the LLM sub-stage was degraded. """ results: list[SarifResult] = [] seen_rule_ids: dict[str, str] = {} @@ -206,6 +216,17 @@ def _build_sarif( for rule_id, description in sorted(seen_rule_ids.items()) ] + invocations: list[SarifInvocation] | None = None + if degraded_notice: + invocations = [ + SarifInvocation( + execution_successful=True, + tool_execution_notifications=[ + SarifNotification(text=SarifMessage(text=degraded_notice), level="warning") + ], + ) + ] + sarif_log = SarifLog( schema_=SARIF_SCHEMA_URI, runs=[ @@ -218,6 +239,7 @@ def _build_sarif( ) ), results=results, + invocations=invocations, ) ], ) @@ -233,6 +255,8 @@ def _format_terminal( risk_severity: str, risk_recommendation: str, has_executable_scripts: bool, + use_llm: bool = True, + llm_call_log: list[dict[str, object]] | None = None, suppressed: list[SuppressedFinding] | None = None, show_suppressed: bool = False, ) -> str: @@ -288,6 +312,17 @@ def _format_terminal( comp_table.add_row(f"... and {len(component_metadata) - 15} more", "", "", "") console.print(comp_table) + degraded_notice = _llm_degradation_notice(use_llm, llm_call_log or []) + if degraded_notice: + console.print() + console.print( + Panel( + f"[bold]Degraded scan[/bold]\n{degraded_notice}", + title="[bold red]WARNING[/bold red]", + border_style="red", + ) + ) + if findings: console.print("\n") console.print(f"[bold]Issues ({len(findings)})[/bold]\n") @@ -327,20 +362,70 @@ def _format_terminal( return console.export_text() -def _build_metadata(has_executable_scripts: bool, use_llm: bool) -> dict[str, object]: +def _llm_runtime_status( + use_llm: bool, llm_call_log: list[dict[str, object]] +) -> tuple[int, int, bool]: + """Return ``(attempted, succeeded, degraded)`` from the LLM call log. + + ``degraded`` is True when the LLM stage was requested and at least one call + was attempted, but every call failed at runtime — meaning the report + reflects static analysis only despite a deep scan being requested. + """ + attempted = len(llm_call_log) + succeeded = sum(1 for r in llm_call_log if r.get("ok")) + degraded = bool(use_llm and attempted > 0 and succeeded == 0) + return attempted, succeeded, degraded + + +def _llm_degradation_notice(use_llm: bool, llm_call_log: list[dict[str, object]]) -> str | None: + """Return a human-readable degraded-scan warning, or None if not degraded.""" + attempted, _succeeded, degraded = _llm_runtime_status(use_llm, llm_call_log) + if not degraded: + return None + return ( + f"LLM analysis was requested but all {attempted} LLM call(s) failed - " + "results reflect STATIC analysis only." + ) + + +def _build_metadata( + has_executable_scripts: bool, + use_llm: bool, + llm_call_log: list[dict[str, object]] | None = None, +) -> dict[str, object]: """Build the metadata section shared by all output formats.""" + llm_call_log = llm_call_log or [] llm_available, llm_error = is_llm_available() - meta_analysis_applied = use_llm and llm_available + attempted, succeeded, degraded = _llm_runtime_status(use_llm, llm_call_log) + # meta_analysis_applied reflects whether the LLM meta-analysis effectively + # ran: requested, available, and not fully degraded (every call failing). + meta_analysis_applied = use_llm and llm_available and not degraded + meta: dict[str, object] = { "has_executable_scripts": has_executable_scripts, "skillspector_version": skillspector_version, "llm_requested": use_llm, - "llm_available": llm_available, + # llm_available reflects runtime truth: the binary/credentials were + # available AND the stage was not fully degraded (every call failing). + "llm_available": llm_available and not degraded, "meta_analysis_applied": meta_analysis_applied, } if not meta_analysis_applied: meta["filtering_mode"] = "heuristic" - if use_llm and not llm_available: + if use_llm and attempted: + meta["llm_calls_attempted"] = attempted + meta["llm_calls_succeeded"] = succeeded + if degraded: + meta["llm_degraded"] = True + reasons = sorted( + {str(r.get("error")) for r in llm_call_log if not r.get("ok") and r.get("error")} + ) + detail = f" Reasons: {'; '.join(reasons)}" if reasons else "" + meta["llm_error"] = ( + f"LLM analysis was requested but all {attempted} LLM call(s) failed; " + f"results reflect static analysis only.{detail}" + ) + elif use_llm and not llm_available: meta["llm_error"] = llm_error return meta @@ -401,6 +486,7 @@ def _format_json( risk_recommendation: str, has_executable_scripts: bool, use_llm: bool = True, + llm_call_log: list[dict[str, object]] | None = None, analysis_completeness: dict[str, object] | None = None, suppressed: list[SuppressedFinding] | None = None, ) -> str: @@ -431,7 +517,7 @@ 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), + "metadata": _build_metadata(has_executable_scripts, use_llm, llm_call_log), } if analysis_completeness is not None: data["analysis_completeness"] = analysis_completeness @@ -447,6 +533,8 @@ def _format_markdown( risk_severity: str, risk_recommendation: str, has_executable_scripts: bool, + use_llm: bool = True, + llm_call_log: list[dict[str, object]] | None = None, suppressed: list[SuppressedFinding] | None = None, show_suppressed: bool = False, ) -> str: @@ -462,6 +550,11 @@ def _format_markdown( lines.append(f"**Scanned:** {datetime.now(UTC).strftime('%Y-%m-%d %H:%M:%S UTC')} ") lines.append("") + degraded_notice = _llm_degradation_notice(use_llm, llm_call_log or []) + if degraded_notice: + lines.append(f"> ⚠️ **Degraded scan:** {degraded_notice}") + lines.append("") + lines.append("## Risk Assessment\n") lines.append("| Metric | Value |") lines.append("|--------|-------|") @@ -541,6 +634,21 @@ def report(state: SkillspectorState) -> dict[str, object]: skill_path = state.get("skill_path") output_format = state.get("output_format") or "sarif" use_llm = state.get("use_llm", True) + llm_call_log = state.get("llm_call_log") or [] + + # Surface a silent degradation: deep scan requested but every LLM call failed + # at runtime, so the report reflects static analysis only. Logged here (once, + # operationally) regardless of output format; also embedded in each format's + # body / metadata below. + _attempted, _succeeded, degraded = _llm_runtime_status(use_llm, llm_call_log) + degraded_notice = _llm_degradation_notice(use_llm, llm_call_log) + if degraded: + logger.warning( + "LLM stage degraded: %d/%d LLM call(s) failed; report reflects static " + "analysis only (llm_available reported false)", + _attempted - _succeeded, + _attempted, + ) baseline = state.get("baseline") show_suppressed = state.get("show_suppressed", False) @@ -554,11 +662,22 @@ def report(state: SkillspectorState) -> dict[str, object]: risk_score, risk_severity, risk_recommendation = _compute_risk_score( findings_for_scoring, has_executable_scripts ) - sarif_report = _build_sarif(active_findings, suppressed) + sarif_report = _build_sarif(active_findings, suppressed, degraded_notice=degraded_notice) analysis_completeness = _build_analysis_completeness( components, file_cache, use_llm, raw_findings, filtered_findings ) + # Fail closed on a degraded deep scan: when the LLM stage was requested but + # every call failed, the semantic analyzers were effectively skipped, so a + # SAFE verdict would rest on static analysis alone. An attacker can trigger + # this on purpose (e.g. content that breaks the LLM call) to dodge semantic + # scrutiny. Floor the recommendation at CAUTION so an install-gate ASKS + # rather than auto-allows; risk_score / severity are left untouched (they + # honestly reflect what static analysis found), and llm_degraded / llm_error + # explain why the verdict was raised. + if degraded and risk_recommendation == "SAFE": + risk_recommendation = "CAUTION" + if output_format == "terminal": report_body = _format_terminal( active_findings, @@ -569,6 +688,8 @@ def report(state: SkillspectorState) -> dict[str, object]: risk_severity, risk_recommendation, has_executable_scripts, + use_llm=use_llm, + llm_call_log=llm_call_log, suppressed=suppressed, show_suppressed=show_suppressed, ) @@ -583,6 +704,7 @@ def report(state: SkillspectorState) -> dict[str, object]: risk_recommendation, has_executable_scripts, use_llm=use_llm, + llm_call_log=llm_call_log, analysis_completeness=analysis_completeness, suppressed=suppressed, ) @@ -596,6 +718,8 @@ def report(state: SkillspectorState) -> dict[str, object]: risk_severity, risk_recommendation, has_executable_scripts, + use_llm=use_llm, + llm_call_log=llm_call_log, suppressed=suppressed, show_suppressed=show_suppressed, ) diff --git a/src/skillspector/providers/__init__.py b/src/skillspector/providers/__init__.py index 307ae6a..1ce9260 100644 --- a/src/skillspector/providers/__init__.py +++ b/src/skillspector/providers/__init__.py @@ -22,12 +22,24 @@ Selection happens via the ``SKILLSPECTOR_PROVIDER`` env var: - openai → OpenAIProvider (api.openai.com) - anthropic → AnthropicProvider (api.anthropic.com) - anthropic_proxy → AnthropicProxyProvider (Vertex-style raw-predict proxy) - nv_build → NvBuildProvider (build.nvidia.com) + openai → OpenAIProvider (api.openai.com) + anthropic → AnthropicProvider (api.anthropic.com) + anthropic_proxy → AnthropicProxyProvider (Vertex-style raw-predict proxy) + nv_build → NvBuildProvider (build.nvidia.com) + claude_cli → ClaudeCLIProvider (local ``claude`` binary, no API key) + codex_cli → CodexCLIProvider (local ``codex`` binary, no API key) + gemini_cli → GeminiCLIProvider (local ``gemini`` binary, no API key) + antigravity_cli → AntigravityCLIProvider (local ``agy`` binary; registered + but disabled — agy is TTY-only and + can't be captured; use gemini_cli) When unset, the selector defaults to ``nv_build``. + +CLI providers (``claude_cli``, ``codex_cli``, ``gemini_cli``) implement the +optional :class:`~skillspector.providers.base.AgentCLICapable` interface — they +expose ``is_available()`` and ``complete()`` so that +:func:`skillspector.llm_utils.get_chat_model` uses the local CLI subprocess +instead of the ``ChatOpenAI`` HTTP transport. """ from __future__ import annotations @@ -37,7 +49,14 @@ from langchain_core.language_models.chat_models import BaseChatModel -from .base import ChatModelProvider, CredentialsProvider, LLMProvider, ModelMetadataProvider +from .base import ( + AgentCLICapable, + ChatModelProvider, + CredentialsProvider, + LLMProvider, + ModelMetadataProvider, + has_cli_capability, +) from .nv_build import NvBuildProvider NO_LLM_API_KEY_MESSAGE = ( @@ -71,6 +90,22 @@ def _select_active_provider() -> LLMProvider: return AnthropicProxyProvider() if name == "nv_build": return NvBuildProvider() + if name == "claude_cli": + from .claude_cli import ClaudeCLIProvider + + return ClaudeCLIProvider() + if name == "codex_cli": + from .codex_cli import CodexCLIProvider + + return CodexCLIProvider() + if name == "gemini_cli": + from .gemini_cli import GeminiCLIProvider + + return GeminiCLIProvider() + if name == "antigravity_cli": + from .antigravity_cli import AntigravityCLIProvider + + return AntigravityCLIProvider() if name in ("nv_inference", ""): # Try the optional nv_inference subpackage if it's bundled with # this installation; otherwise fall through to nv_build. @@ -83,7 +118,8 @@ def _select_active_provider() -> LLMProvider: raise ValueError( f"Unknown SKILLSPECTOR_PROVIDER: {name!r}. " - "Expected one of: openai, anthropic, anthropic_proxy, nv_build (or unset)." + "Expected one of: openai, anthropic, anthropic_proxy, nv_build, " + "claude_cli, codex_cli, gemini_cli, antigravity_cli (or unset)." ) @@ -92,11 +128,22 @@ def get_metadata_provider() -> ModelMetadataProvider: return _select_active_provider() +def get_active_provider() -> ModelMetadataProvider: + """Return the active provider (alias for :func:`get_metadata_provider`). + + Preferred over :func:`get_metadata_provider` when callers also need to + check for optional capabilities (e.g. :func:`has_cli_capability`). + """ + return _select_active_provider() + + def resolve_provider_credentials() -> tuple[str, str | None] | None: """Return ``(api_key, base_url)`` from the active provider. Returns ``None`` when the provider's credential env var is unset, so - callers can fall through to other credential sources. + callers can fall through to other credential sources. CLI providers + always return ``None`` from this method; availability is checked via + ``is_available()`` instead. """ return _select_active_provider().resolve_credentials() @@ -125,37 +172,48 @@ def create_chat_model( ) -> BaseChatModel: """Create the active provider's native LangChain chat model. + CLI providers (``claude_cli``, ``codex_cli``, ``gemini_cli``) do not have + a native LangChain chat model — callers that need CLI transport should use + :func:`skillspector.llm_utils.get_chat_model` instead (which returns an + :class:`~skillspector.llm_utils.AgentCLIChatModel` adapter). + If the active provider is not configured, fall back to standard OpenAI environment variables. This preserves the historical ``OPENAI_API_KEY`` escape hatch while letting configured providers choose their own client. """ provider = _select_active_provider() - llm = provider.create_chat_model(model, max_tokens=max_tokens, timeout=timeout) - if llm is not None: - return llm - - from .openai import OpenAIProvider - if not isinstance(provider, OpenAIProvider): - llm = _openai_fallback_provider().create_chat_model( - model, - max_tokens=max_tokens, - timeout=timeout, - ) + # CLI providers don't participate in the create_chat_model path. + 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 + from .openai import OpenAIProvider + + if not isinstance(provider, OpenAIProvider): + llm = _openai_fallback_provider().create_chat_model( + model, + max_tokens=max_tokens, + timeout=timeout, + ) + if llm is not None: + return llm + raise_no_llm_api_key_configured() __all__ = [ + "AgentCLICapable", "ChatModelProvider", "CredentialsProvider", "LLMProvider", "ModelMetadataProvider", "NO_LLM_API_KEY_MESSAGE", "create_chat_model", + "get_active_provider", "get_metadata_provider", + "has_cli_capability", "raise_no_llm_api_key_configured", "resolve_chat_model_credentials", "resolve_provider_credentials", diff --git a/src/skillspector/providers/_agent_cli.py b/src/skillspector/providers/_agent_cli.py new file mode 100644 index 0000000..d7aa415 --- /dev/null +++ b/src/skillspector/providers/_agent_cli.py @@ -0,0 +1,805 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Hardened subprocess helper for agent CLI providers (claude, codex, gemini). + +This is the single security chokepoint for all agent-CLI calls. Per-CLI +knowledge (argv, output parsing, auth check) lives in a small ``CliSpec`` +registry (see ``_REGISTRY`` / "HOW TO ADD A NEW AGENT CLI" below); the +security core is CLI-agnostic. Every call goes through :func:`run_agent_cli` +which enforces: + +- **No shell**: ``shell=False`` with an explicit argv list. +- **Untrusted content via stdin only**: the prompt (which may contain + adversarial skill content) is written to the process stdin, never + injected into argv. +- **Capability stripping** (per-binary): tools disabled, MCP disabled, + no extra directories, deny permission mode (claude); read-only sandbox + (codex). ``--dangerously-skip-permissions`` is NEVER used. +- **Environment scrubbing**: API keys, SSH keys, cloud credentials, and + other secrets are stripped from the child environment. +- **Timeout enforcement**: the call raises ``TimeoutError`` rather than + hanging indefinitely. +- **Input / output caps**: prompt exceeding ``MAX_INPUT_BYTES`` is + rejected; stdout is capped at ``MAX_OUTPUT_BYTES``. +- **Fail-closed**: non-zero exit, timeout, missing binary, or bad + output all raise ``AgentCLIError``. +- **Prompt-layer hardening**: the caller wraps untrusted content in + clear DATA delimiters before passing it here (defense-in-depth on top + of capability removal). + +The JSON output envelope (``claude -p --output-format json``) is parsed +and the assistant text is returned. ``codex exec --json`` produces +JSONL events; the last assistant message is extracted. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import tempfile +import threading +from collections.abc import Callable +from dataclasses import dataclass +from typing import Any + +from skillspector.logging_config import get_logger + +logger = get_logger(__name__) + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +# Reuse the same cap as static_runner so a skill that's too big for static +# analysis is also too big to send to the CLI. +MAX_INPUT_BYTES = 1_000_000 # 1 MB — mirrors MAX_FILE_BYTES in static_runner.py +MAX_OUTPUT_BYTES = 10_000_000 # 10 MB safety cap on stdout +MAX_STDERR_BYTES = 64_000 # stderr is only used for error snippets +CLI_TIMEOUT_SECONDS = 300 # 5-minute per-call hard limit + +# Environment variables that must NOT be forwarded to child processes. +# Includes API keys, cloud creds, SSH agent, and SkillSpector's own keys. +_SECRET_ENV_PREFIXES: tuple[str, ...] = ( + "ANTHROPIC_API_KEY", + "OPENAI_API_KEY", + "NVIDIA_INFERENCE_KEY", + "NVIDIA_INFERENCE_METADATA_KEY", + "AWS_", + "AZURE_", + "GOOGLE_", + "GCLOUD_", + "GCP_", + "SSH_", + "GPG_", + "GITHUB_TOKEN", + "GITLAB_TOKEN", + "HUGGINGFACE_TOKEN", + "HF_TOKEN", + "COHERE_API_KEY", + "REPLICATE_API_TOKEN", + "MISTRAL_API_KEY", + "TOGETHER_API_KEY", + "GROQ_API_KEY", + "FIREWORKS_API_KEY", + "LANGCHAIN_API_KEY", + "LANGSMITH_API_KEY", +) + + +class AgentCLIError(RuntimeError): + """Raised when an agent CLI call fails for any reason (fail-closed).""" + + +# --------------------------------------------------------------------------- +# Environment scrubbing +# --------------------------------------------------------------------------- + + +def _scrub_env() -> dict[str, str]: + """Return a copy of ``os.environ`` with secret variables removed. + + Any variable whose name starts with a prefix in ``_SECRET_ENV_PREFIXES`` + is stripped. The resulting environment is passed to the subprocess. + """ + clean: dict[str, str] = {} + for key, val in os.environ.items(): + upper = key.upper() + if any(upper.startswith(p.upper()) for p in _SECRET_ENV_PREFIXES): + continue + clean[key] = val + return clean + + +# --------------------------------------------------------------------------- +# Binary lookup +# --------------------------------------------------------------------------- + + +def find_binary(name: str) -> str | None: + """Return the absolute path of *name* on PATH, or ``None`` if absent.""" + return shutil.which(name) + + +# --------------------------------------------------------------------------- +# Argument validation +# --------------------------------------------------------------------------- + + +def _validate_model_label(model: str) -> str: + """Ensure *model* cannot be used as an argument injection vector. + + Model labels come from ``SKILLSPECTOR_MODEL`` (user-controlled) or the + provider's defaults. We verify the label does not start with ``-`` + (which would look like a flag to the CLI) and contains only safe + characters. + + Raises: + AgentCLIError: when the label fails validation. + """ + if not model: + raise AgentCLIError("model label must be a non-empty string") + if model.startswith("-"): + raise AgentCLIError( + f"model label {model!r} starts with '-'; this looks like an argument injection attempt" + ) + # Allow alphanumeric, dash, dot, slash, colon, underscore (covers all + # known claude/codex model identifiers). + allowed = set("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-./: _") + bad = [c for c in model if c not in allowed] + if bad: + raise AgentCLIError(f"model label {model!r} contains disallowed characters: {bad!r}") + return model + + +# --------------------------------------------------------------------------- +# Claude CLI invocation +# --------------------------------------------------------------------------- + + +def _build_claude_argv(binary: str, model: str, max_output_tokens: int) -> list[str]: + """Build the argv list for a capability-stripped ``claude -p`` call. + + ``-p`` / ``--print`` + Non-interactive single-shot mode. The prompt is read from stdin; + the response is written to stdout and the process exits. + + ``--output-format text`` + Emit the assistant's response as plain text — nothing else. This is + the most stable format the claude CLI offers: it has been the canonical + headless contract since ``-p`` was introduced, predates the JSON + envelope formats, and is unaffected by changes to the event-stream + schema. The envelope formats (``json`` / ``stream-json``) have changed + shape across builds (single dict → JSON array → JSONL); ``text`` never + has. Because we need only the response text and not the metadata + (session ID, stop reason, etc.) that the envelope carries, ``text`` is + the right choice here: the format we request defines exactly what we + parse, with no version detection and no fallbacks. + + ``--model