feat(providers): local agent-CLI providers (claude/codex/gemini), no API key

[AbhiramDwivedi]

Closes #57

What

Local agent-CLI providers that route Stage-2 LLM analysis through a locally-installed, already-authenticated agent CLI instead of a metered HTTP API — no API key needed, the CLI's own login session is used. Supported today: claude, codex, gemini (via SKILLSPECTOR_PROVIDER=claude_cli / codex_cli / gemini_cli). Adding another CLI is a small registry entry + a ~5-line provider subclass — see the "HOW TO ADD A NEW AGENT CLI" guide in providers/_agent_cli.py.

How

• One hardened chokepoint (providers/_agent_cli.py): shell=False; untrusted prompt via stdin only, never argv; capability stripping verified against the real CLIs — claude: --allowed-tools "" deny-by-default + --permission-mode dontAsk + --strict-mcp-config + --disable-slash-commands; codex: --sandbox read-only + --ephemeral + --ignore-user-config; gemini: --approval-mode plan (read-only, no tool execution). --dangerously-skip-permissions / auto-approve is never used. Scrubbed env; temp CWD; per-call timeout; streamed stdout with a hard size cap (process killed on overflow — no unbounded buffering); model-label validated against argument injection; fail-closed on every error path. A per-CLI CliSpec registry keeps all CLI-specific argv/parse/auth behind one lookup, so the shared security core is unchanged when a CLI is added.
• The analyzer seam is untouched. LLM analyzers get their model from get_chat_model(); for CLI providers that returns a minimal ChatOpenAI-compatible adapter backed by provider.complete(), with structured output via prompt-for-JSON + Pydantic validation (fail-closed). HTTP providers are unchanged.
• No pinned model versions. A CLI provider forwards no --model by default, so it runs with the user's OWN configured model and thinking level; set SKILLSPECTOR_MODEL to override. (No bundled model_registry.yaml; CLI providers use the package-wide default token budgets.)
• is_available() does a real local auth probe (claude auth status / codex login status) so a report's llm_available never claims availability when the CLI is logged out.

Antigravity (agy) — registered but disabled

antigravity_cli is wired into the registry but fail-closed and disabled. Tested end-to-end against the real agy: its --print mode renders to a TTY and returns empty stdout on a pipe (how the runner must capture it), and it takes the prompt as an argv value rather than stdin — so it cannot be driven programmatically. Its backend is Gemini, so use gemini_cli for that capability. The registry entry documents the finding in one place; enabling it later is a one-function change if agy gains a headless stdout mode.

Test

Fully-mocked unit tests (no CLI required) cover the subprocess invariants, the bounded reader (real subprocesses: normal / overflow-kill / timeout), the adapter + structured-output parsing, provider selection, the registry, no-pinned-model resolution, and the disabled-antigravity guard — so a contributor without any of these CLIs installed runs the entire default suite. An opt-in live harness (tests/integration/test_agent_cli_live.py, marked integration, excluded by default) is parametrized over claude/codex/gemini and skips per-CLI when a binary is absent/unauthenticated. Verified end-to-end: claude/codex/gemini each return real output with no pinned model.

🤖 Generated with Claude Code

[AbhiramDwivedi]

Rebased onto the latest main (a5092dd) and adapted onto the recent provider refactor (the new providers/chat_models.py + create_chat_model() seam), so this is mergeable again. Collapsed to a single commit.

Integration summary

• HTTP providers now flow through your create_chat_model() (native ChatAnthropic, etc.); the CLI providers branch to a small AgentCLIChatModel adapter selected by a duck-typed has_cli_capability() check — the existing HTTP path is unchanged.
• is_llm_available() delegates to the provider's is_available() for CLI providers and falls back to credential resolution for HTTP providers.
• No model pinned: CLI providers forward no --model (the user's own CLI-configured model is used); SKILLSPECTOR_MODEL overrides.

Re-verified end-to-end after the rebase: claude, codex, and gemini each return a live response through the full chat_completion → AgentCLIChatModel → CLI path (both plain and structured-output), with no model pinned. Unit suite + ruff green; the hardened subprocess chokepoint (providers/_agent_cli.py) is unchanged from the pre-rebase commit.

[486]

claude_cli provider: _parse_claude_output rejects the array-shaped --output-format json emitted by some Claude Code builds

First, thanks for this PR — the hardened _agent_cli.py chokepoint (stdin-only untrusted content, capability stripping, env scrub, temp CWD, fail-closed) is exactly what's needed to point an LLM at adversarial skill content. Tried it end-to-end with SKILLSPECTOR_PROVIDER=claude_cli and hit one CLI-version compatibility issue worth flagging.

Symptom

With a Claude Code CLI at 2.1.178, every Stage-2 semantic analyzer fails and the scan silently degrades to static-only:

WARNING semantic_security_discovery failed: expected a JSON object from claude, got list: '[{"type":"system","subtype":"init", ...
WARNING semantic_developer_intent  failed: expected a JSON object from claude, got list: ...
WARNING semantic_quality_policy    failed: expected a JSON object from claude, got list: ...

Root cause

On this build, claude -p --output-format json returns a JSON array of stream events rather than a single result object. The assistant text lives in the final element (type == "result"):

$ printf 'Reply OK.' | claude -p --output-format json --allowed-tools "" \
    --permission-mode dontAsk --strict-mcp-config --disable-slash-commands \
  | python3 -c "import json,sys; d=json.load(sys.stdin); \
      print(type(d).__name__, [x.get('type') for x in d])"
list ['system', 'system', 'system', 'system', 'system', 'system', 'system', 'assistant', 'assistant', 'result']

_parse_claude_output only handles the single-dict envelope, so it raises expected a JSON object from claude, got list. The analyzers catch this and fall back to static-only, which is easy to miss unless you check the warnings.

Suggested fix — accept both shapes in _parse_claude_output: if the parsed JSON is a list, select the last element carrying a result key (preferring type == "result"), then continue with the existing dict handling:

    if isinstance(envelope, list):
        result_event = None
        for item in envelope:
            if isinstance(item, dict) and "result" in item:
                if str(item.get("type", "")).lower() == "result":
                    result_event = item
                elif result_event is None:
                    result_event = item
        if result_event is None:
            raise AgentCLIError(
                f"claude event array has no element with a 'result' key; "
                f"types={[i.get('type') for i in envelope if isinstance(i, dict)]!r}"
            )
        envelope = result_event

After this change the semantic analyzers run and metadata.llm_available == true, verified against a batch of real skills.

It may also be worth surfacing a partial-degradation signal (e.g. flip llm_available to false or warn loudly at the end) when the LLM stage was requested but every analyzer failed, so a parser/transport break can't quietly turn a deep scan into a static-only one.

Happy to open a PR with the parser change + a unit test for the array shape if useful.

[AbhiramDwivedi]

@486 — thanks for the detailed report; the repro and the types=[...] dump made this trivial to confirm, and your second point turned out to be the more important one.

1. The parser break — fixed at the root (42e75fe)

You're right that _parse_claude_output only handled the single-object envelope. Rather than special-case the array shape, I stepped back: --output-format json has never had a stable cross-version contract (single object in 2.1.177, your event array in 2.1.178, JSONL in others), and we don't actually need the envelope — only the assistant text. So the provider now requests --output-format text, the stable headless contract since -p was introduced. _parse_claude_output collapses to strip-and-return; the structured output the analyzers need is still produced by the model (schema appended to the prompt) and validated by Pydantic in the adapter, so it's unaffected by the transport change. No version detection, no fallback cascade. (Same pass deduped a copy-pasted agy parser and re-verified agy 1.0.10 is still TTY-only/uncapturable.)

2. Partial-degradation signal — did both (ad33dba, 127bc24)

Agreed this is the dangerous part. Each LLM-backed node (the 3 semantic analyzers, the meta-analyzer, and mcp_tool_poisoning's TP4) now records a per-run entry in state['llm_call_log'] (accumulated via the same operator.add reducer as findings). When use_llm was requested but every call failed, the report:

• flips metadata.llm_available to false (your suggestion), and
• warns loudly on every surface: llm_degraded: true + llm_calls_attempted/succeeded + deduped reasons in JSON, a warning panel in terminal, a blockquote in markdown, a toolExecutionNotifications entry in the default SARIF output, and one aggregate log line.

llm_available now means "LLM analysis actually contributed," not just "the CLI was reachable."

Two things that fell out of implementing it:

• An end-to-end test (LLM mocked to fail) caught a latent crash: meta_analyzer constructed its chat model outside its try/except, so a construction failure took down the whole graph instead of degrading. Fixed (2eb03c4).
• Fail-closed follow-through (ad3a717): a deliberately-degraded scan could still report SAFE/exit 0 from static findings alone — an evasion path. A degraded scan now floors the recommendation at CAUTION so a gate asks rather than auto-allows; risk_score stays honest and llm_degraded explains the bump.

Happy to split any of this into its own PR if you'd prefer to keep #52 scoped to the providers — just say the word.

[rng1995]

Review: APPROVE

This is a large but cohesive and unusually well-engineered feature. I read the security-critical paths closely (_agent_cli.py, the llm_utils adapter, provider wiring, and the report.py degradation handling) and the posture is excellent.

Security (the part that matters most for this tool)

• Single chokepoint (run_agent_cli) with shell=False and an explicit argv list; untrusted prompt content is delivered via stdin only and never reaches argv. The model label is the only dynamic argv element and it's validated (_validate_model_label rejects leading - and restricts the charset), closing the obvious argument-injection vector.
• Capability stripping is per-CLI and deny-by-default: claude --allowed-tools "" --permission-mode dontAsk --strict-mcp-config --disable-slash-commands, codex --sandbox read-only --ephemeral --ignore-user-config --ignore-rules, gemini --approval-mode plan; --dangerously-skip-permissions/auto-approve are never used. Good that this is asserted in tests rather than just documented.
• Defense-in-depth is thorough: scrubbed env, fresh temp CWD, hard timeout, bounded concurrent stdout/stderr draining with an overflow-kill (no unbounded buffering), and fail-closed on every error path (missing binary, non-zero exit, timeout, empty/garbage output).
• The disabled antigravity_cli fails closed in _build_agy_argv and is_available(), which is the right way to register-but-disable an incompatible CLI.
• Nice touch: report.py floors a SAFE verdict to CAUTION when the LLM stage was requested but every call failed, so an attacker can't dodge semantic scrutiny by breaking the LLM call. That's a genuine fail-closed improvement, and the blocking-verdict case is preserved (tested).

Correctness / quality

• Provider selection, the AgentCLICapable duck-typed capability check, and the get_chat_model dispatch are consistent with the existing provider abstraction; HTTP providers are untouched.
• The with_structured_output adapter augments the prompt with the schema and validates via Pydantic, fail-closing (ValueError) on unparseable output; _extract_json_object handles fenced/prose-wrapped JSON.
• Output parsers are version-tolerant (codex nested/flat shapes, gemini key fallbacks) with empty-output raising.

Tests — strong and on-point: the subprocess invariants (shell=False, stdin-not-argv, injection-stays-on-stdin, no dangerous flags, env scrubbed, input-size guard, overflow-kill, timeout), model-label validation, registry, adapter structured-output fail-closed, and the degraded-scan matrix are all covered. Keeping the live CLI suite opt-in/integration so the default suite runs without any CLI installed is the right call.

Minor / optional (non-blocking)

• The secret-env scrub is a prefix denylist, so provider tokens that don't match a listed prefix (e.g. GEMINI_API_KEY, ANTHROPIC_AUTH_TOKEN) are still forwarded. Given tools are disabled / sandbox is read-only the model can't read env anyway, so this is defense-in-depth only — but an allowlist (forward only what the CLIs need) would be strictly tighter and wouldn't silently miss new tokens.
• codex --sandbox read-only still permits filesystem reads by model-generated commands (claude/gemini block tool use entirely). It's the most restrictive codex mode available and there's no attacker exfil channel here (output returns to the operator's own report, no network egress), so this is informational rather than a gap — worth a one-line note in the codex provider docstring.
• AgentCLIChatModel implements only the invoke/ainvoke/with_structured_output slice of BaseChatModel. That matches current analyzer usage, but any future analyzer that reaches for .batch()/.stream()/callbacks will break under CLI providers — a comment or a NotImplementedError stub would make that boundary explicit.
• In _run_bounded, the drain threads are joined with a 5s timeout before reading the buffers; in a pathological case where a thread outlives the join the captured stdout could be truncated. The process has already exited (pipe EOF) by then, so this is very unlikely, just flagging it.

Given the scope, a reviewer with the CLIs installed running the opt-in live harness once would be a good final gate, but nothing here blocks. Excellent work — approving.

[AbhiramDwivedi]

@rng1995 Thanks for the thorough security review. Addressed the two cheap clarity notes in 714e266:

• codex read-only docstring — expanded the --sandbox read-only section to note that, unlike claude/gemini (which block model tool use entirely), codex's strictest mode still permits read-only filesystem reads by model-generated commands. Informational only: isolated empty temp CWD, output returns to the operator's own report, no network egress.
• AgentCLIChatModel boundary — batch()/stream() now raise NotImplementedError (typed -> NoReturn), so a future analyzer reaching for the unsupported BaseChatModel surface fails loudly with a clear message instead of a confusing AttributeError. No behavior change for the existing invoke/ainvoke/with_structured_output callers.

I left the env-scrub denylist and the codex read-only behavior itself as-is, per your "informational, not a gap" framing (tools disabled / sandbox read-only, no exfil channel). Happy to switch the scrub to an allowlist as a follow-up if you'd prefer the strictly-tighter version.

[rng1995]

Looks good to me. Please resolve merge conflicts to merge the PR.

[rng1995]

[Automated SkillSpector Review]

Re-review — re-confirming approval.

The one commit since my approval is documentation + a clarity hardening that addresses two of my non-blocking notes: the codex --sandbox read-only docstring now states it still permits model-driven filesystem reads (unlike claude/gemini), and AgentCLIChatModel.batch()/stream() now raise NotImplementedError (-> NoReturn) so a future analyzer reaching for the unsupported BaseChatModel surface fails loudly instead of with a confusing AttributeError. No behavior change to the security-critical run_agent_cli chokepoint (still shell=False, stdin-only untrusted input, validated model label, deny-by-default capability stripping, fail-closed). Approving.

[rng1995]

Please resolve merge conflicts

[AbhiramDwivedi]

@rng1995 Resolved conflicts against the latest main (merged it in; 8 files). Both feature sets are preserved side by side — this PR's CLI providers + the LLM silent-degradation signal, and main's anthropic_proxy provider, the provider model-resolution refactor, baseline/suppression, and SARIF rules[].

A few notable resolutions:

• get_chat_model: the CLI branch now takes its default model from provider.resolve_model() (since MODEL_CONFIG was removed upstream). For CLI providers that returns "", so --model stays omitted and the user's own CLI-configured model is used — no pinned version. The HTTP branch keeps your _resolve_default_chat_model().
• meta_analyzer failure path: kept your fail-closed _passthrough_with_defaults and this PR's ok=False degradation record.
• report: _build_sarif now takes both suppressed and degraded_notice; _build_metadata merges meta_analysis_applied/filtering_mode with the degradation fields (a degraded scan is no longer counted as meta_analysis_applied).

Full unit suite is green (1123 passed). One heads-up that's independent of this PR: test_rejected_finding_still_dropped and test_low_confidence_finding_dropped currently fail on main itself — PR #54's severity floor keeps CRITICAL/HIGH findings, but those two tests assert a CRITICAL finding gets dropped. I adjusted them to use a MEDIUM finding so they exercise the drop path (the CRITICAL/HIGH floor stays covered in test_llm_analyzer_base). Happy to split that into its own PR against main if you'd prefer. Ready for re-merge.