diff --git a/CHANGELOG.md b/CHANGELOG.md index b924b4113..a8e7c8a84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ### 2.12.0 (Unreleased) ### Features/Bug Fixes +* feat(providers): add copilot_cli agent-CLI LLM provider * fix(security): retain incomplete coverage for runtime-selected commands and remeasure active Git clones strictly (#514) * fix(scan): preserve required-input failures and multiline prompt uncertainty (#563) * fix(analyzer): preserve Perl print literal ownership and explain referenced-artifact limitations (#615) diff --git a/README.md b/README.md index 68d5dfc54..5373e90a8 100644 --- a/README.md +++ b/README.md @@ -243,6 +243,7 @@ inference gateways. | `codex_cli` | _(none — uses local CLI auth)_ | local `codex` binary | local Codex runtime fallback, or `SKILLSPECTOR_MODEL` | | `gemini_cli` | _(none — uses local CLI auth)_ | local `gemini` binary | local Gemini runtime fallback, or `SKILLSPECTOR_MODEL` | | `opencode_cli` | _(none — uses local CLI auth)_ | local `opencode` 1.18.32 binary | local OpenCode runtime fallback, or `SKILLSPECTOR_MODEL` | +| `copilot_cli` | _(none — uses local CLI auth)_ | local `copilot` binary | local Copilot runtime fallback, or `SKILLSPECTOR_MODEL` | Structured output is requested through LangChain's `with_structured_output`, whose default forces a tool call. Some models reject a forced tool call with @@ -306,6 +307,13 @@ skillspector scan ./my-skill/ export SKILLSPECTOR_PROVIDER=codex_cli skillspector scan ./my-skill/ +# Local GitHub Copilot CLI — no API key; uses your existing `copilot login` session +# Requires: copilot CLI installed and authenticated +export SKILLSPECTOR_PROVIDER=copilot_cli +# Uses the local Copilot CLI runtime fallback unless SKILLSPECTOR_MODEL is set. +# export SKILLSPECTOR_MODEL=claude-sonnet-5 +skillspector scan ./my-skill/ + # Gemini (via OpenAI compatibility layer) export SKILLSPECTOR_PROVIDER=openai export OPENAI_API_KEY="YOUR_GEMINI_API_KEY" @@ -621,7 +629,7 @@ Issues (2) | Variable | Description | Required | |----------|-------------|----------| -| `SKILLSPECTOR_PROVIDER` | Active LLM provider: `openai`, `anthropic`, `anthropic_proxy`, `bedrock`, `nv_build`, `ollama`, `azure_openai`, `openai_compatible`, `claude_cli`, `codex_cli`, `gemini_cli`, or `opencode_cli`. Hosted providers use bundled `model_registry.yaml` defaults; CLI providers fall back to the local runtime's default model unless `SKILLSPECTOR_MODEL` is set. Defaults to `nv_build`. | Optional | +| `SKILLSPECTOR_PROVIDER` | Active LLM provider: `openai`, `anthropic`, `anthropic_proxy`, `bedrock`, `nv_build`, `ollama`, `azure_openai`, `openai_compatible`, `claude_cli`, `codex_cli`, `gemini_cli`, `opencode_cli`, or `copilot_cli`. Hosted providers use bundled `model_registry.yaml` defaults; CLI providers fall back to the local runtime's default model unless `SKILLSPECTOR_MODEL` is set. 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 | @@ -648,7 +656,7 @@ 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`, `gemini_cli`, `opencode_cli`): No API key is needed. Authentication is managed entirely by the agent CLI's own login session. SkillSpector never reads or forwards API keys when these providers are active. The subprocess is run with capabilities restricted, and untrusted skill content is delivered only via stdin. +> **CLI providers** (`claude_cli`, `codex_cli`, `gemini_cli`, `opencode_cli`, `copilot_cli`): No API key is needed. Authentication is managed entirely by the agent CLI's own login session. SkillSpector never reads or forwards API keys when these providers are active. The subprocess is run with capabilities restricted, and untrusted skill content is delivered only via stdin. > > `opencode_cli` currently fails closed unless the installed OpenCode version is exactly `1.18.32`, the version whose configuration precedence and deny-all semantics are verified by this release. diff --git a/docs/release/skillspector-2.12.0.md b/docs/release/skillspector-2.12.0.md index b22d27c8d..579b21a7b 100644 --- a/docs/release/skillspector-2.12.0.md +++ b/docs/release/skillspector-2.12.0.md @@ -44,6 +44,7 @@ The candidate also includes the previously pending Markdown-reference and runtim - HTTPS GitHub `/tree/` URLs can select an existing repository subdirectory. Resolution uses the longest advertised branch/tag name, including refs containing slashes, and rejects decoded traversal or out-of-checkout selections. Arbitrary commit-SHA tree URLs are not supported by this ref-resolution path ([#561](https://github.com/NVIDIA/SkillSpector/pull/561)). - `SKILLSPECTOR_COMPACT_PROMPTS=1`, `true`, or `yes` enables unpadded LLM prompt line labels such as `L1:` instead of `L01:`. Values are trimmed and case-insensitive; unset and other values retain the existing format ([#542](https://github.com/NVIDIA/SkillSpector/pull/542)). - `SKILLSPECTOR_PROVIDER=opencode_cli` runs semantic analysis through a local OpenCode login. The verified deny-all policy requires exactly OpenCode 1.18.31; authentication, version, policy, empty-output, or event-envelope failures fail closed. `SKILLSPECTOR_MODEL` remains optional ([#536](https://github.com/NVIDIA/SkillSpector/pull/536), [#575](https://github.com/NVIDIA/SkillSpector/pull/575)). +- `SKILLSPECTOR_PROVIDER=copilot_cli` runs semantic analysis through a local GitHub Copilot CLI login. The prompt is piped to stdin, `--available-tools` acts as an exclusive allowlist that strips every built-in and MCP tool, and `--disable-builtin-mcps`/`--disallow-temp-dir` drop the bundled GitHub MCP server and the implicit temp-dir grant. Authentication, empty-output, missing-assistant-message, and non-zero result-event failures fail closed. No model is pinned; `SKILLSPECTOR_MODEL` remains optional. - TP4 semantic analysis includes non-empty, closed Markdown/text fences with recognized executable-language labels, using the validated provider-eligible cache and original document line numbers. Size, count, prompt, and runtime limits remain explicit coverage constraints ([#421](https://github.com/NVIDIA/SkillSpector/pull/421)). - The repository-provided OpenCode extension adds a static-by-default `/skillspector` command and `skillspector_scan` tool. Copy `.opencode/` from a checkout to install it; the wheel does not install the extension. It resolves the binary from `SKILLSPECTOR_BIN`, a worktree `.venv`, or `PATH`, requests required host capabilities before launch, rejects symlinked target/output/binary paths, and uses a 120-second timeout with bounded, redacted output. Semantic analysis is opt-in through `noLlm=false` and the provider environment ([#537](https://github.com/NVIDIA/SkillSpector/pull/537)). - `SKILLSPECTOR_MAX_STATIC_ANALYSIS_SECONDS_PER_ARTIFACT` configures the static pattern and YARA time allowance per artifact; its default increases from 30 to 300 seconds. The remaining workflow deadline still bounds both analyzers. diff --git a/src/skillspector/cli.py b/src/skillspector/cli.py index 8c146c4d8..957904e6e 100644 --- a/src/skillspector/cli.py +++ b/src/skillspector/cli.py @@ -561,7 +561,8 @@ def scan( anthropic_proxy | bedrock | nv_build | nv_inference | ollama | azure_openai | openai_compatible | claude_cli | codex_cli | - gemini_cli | opencode_cli. Defaults to the NVIDIA path + gemini_cli | opencode_cli | copilot_cli. + Defaults to the NVIDIA path (nv_inference, falling back to nv_build in OSS builds). SKILLSPECTOR_MODEL Override the active provider's default @@ -584,7 +585,7 @@ def scan( SKILLSPECTOR_COMPAT_BASE_URL for openai_compatible ollama uses the local Ollama service. claude_cli, codex_cli, - gemini_cli, and opencode_cli use their CLI's existing local + gemini_cli, opencode_cli, and copilot_cli use their CLI's existing local authentication session. """ if mcp_registry: diff --git a/src/skillspector/inference_usage.py b/src/skillspector/inference_usage.py index ca670e392..4a53dacc2 100644 --- a/src/skillspector/inference_usage.py +++ b/src/skillspector/inference_usage.py @@ -333,6 +333,7 @@ def provider_name(provider: object) -> str: "OpenAICompatibleProvider": "openai_compatible", "OpenAIProvider": "openai", "OpencodeCLIProvider": "opencode_cli", + "CopilotCLIProvider": "copilot_cli", } return names.get(type(provider).__name__, _label(type(provider).__name__.lower())) diff --git a/src/skillspector/providers/__init__.py b/src/skillspector/providers/__init__.py index 754fc2c5a..c5c6cbd93 100644 --- a/src/skillspector/providers/__init__.py +++ b/src/skillspector/providers/__init__.py @@ -39,7 +39,7 @@ When unset, the selector defaults to ``nv_build``. -CLI providers (``claude_cli``, ``codex_cli``, ``gemini_cli``, ``opencode_cli``) implement the +CLI providers (``claude_cli``, ``codex_cli``, ``copilot_cli``, ``gemini_cli``, ``opencode_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 @@ -156,6 +156,10 @@ def _select_active_provider() -> LLMProvider: from .opencode_cli import OpencodeCLIProvider return OpencodeCLIProvider() + if name == "copilot_cli": + from .copilot_cli import CopilotCLIProvider + + return CopilotCLIProvider() if name == "antigravity_cli": from .antigravity_cli import AntigravityCLIProvider @@ -174,7 +178,8 @@ def _select_active_provider() -> LLMProvider: f"Unknown SKILLSPECTOR_PROVIDER: {name!r}. " "Expected one of: openai, anthropic, anthropic_proxy, bedrock, nv_build, " "ollama, azure_openai, openai_compatible, " - "claude_cli, codex_cli, gemini_cli, opencode_cli, antigravity_cli (or unset)." + "claude_cli, codex_cli, gemini_cli, opencode_cli, copilot_cli, antigravity_cli " + "(or unset)." ) @@ -255,7 +260,7 @@ def create_chat_model_with_provider( ) -> tuple[BaseChatModel, LLMProvider]: """Create a chat model and return the provider that actually built it. - CLI providers (``claude_cli``, ``codex_cli``, ``gemini_cli``, + CLI providers (``claude_cli``, ``codex_cli``, ``copilot_cli``, ``gemini_cli``, ``opencode_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 diff --git a/src/skillspector/providers/_agent_cli.py b/src/skillspector/providers/_agent_cli.py index 65bd0b67e..cc3fffdd6 100644 --- a/src/skillspector/providers/_agent_cli.py +++ b/src/skillspector/providers/_agent_cli.py @@ -874,6 +874,112 @@ def _opencode_auth_check(binary: str) -> tuple[bool, str | None]: return True, None +# --------------------------------------------------------------------------- +# GitHub Copilot CLI invocation (verified against GitHub Copilot CLI 1.0.88) +# --------------------------------------------------------------------------- + +# A tool allowlist entry that matches no real tool. Copilot's --available-tools +# is an exclusive allowlist: every tool not named is removed from the model's +# tool set, so an intentionally unmatchable name strips ALL tools (built-ins and +# MCP tools alike). Verified: the CLI reports every tool in "Disabled tools". +_COPILOT_NO_TOOLS_SENTINEL = "skillspector-no-tools" + + +def _build_copilot_argv(binary: str, model: str, max_output_tokens: int = 0) -> list[str]: + """Build a capability-stripped, non-interactive Copilot CLI argv. + + Flags chosen (verified end-to-end against GitHub Copilot CLI 1.0.88): + + no ``-p`` + The prompt is piped to stdin; Copilot reads piped stdin as the prompt, + so untrusted content never reaches argv. + + ``--available-tools=`` + Exclusive allowlist naming no real tool — removes every built-in and + MCP tool from the model. This is the primary capability removal. + ``--allow-all-tools`` / ``--allow-all`` / ``--yolo`` (auto-approve) and + ``--allow-all-paths`` / ``--allow-all-urls`` are deliberately NEVER used. + + ``--disable-builtin-mcps`` / ``--disallow-temp-dir`` + Drops the bundled GitHub MCP server and the implicit temp-dir grant. + + ``--output-format=json`` / ``--stream=off`` / ``-s`` / ``--log-level=none`` + Structured JSONL on stdout with no progress chrome to parse around. + + Variadic options are passed in ``--flag=value`` form so they cannot swallow + the flags that follow them. + """ + # --model omitted by default -> copilot uses the user's own configured model + # (forwarded only when SKILLSPECTOR_MODEL is set). + model_arg = ["--model", _validate_model_label(model)] if model else [] + return [ + binary, + "-s", # response only, no stats banner + f"--available-tools={_COPILOT_NO_TOOLS_SENTINEL}", # no tool execution + "--disable-builtin-mcps", + "--disallow-temp-dir", + "--stream=off", + "--output-format=json", + "--log-level=none", + *model_arg, + ] + + +def _parse_copilot_output(raw: str) -> str: + """Extract assistant text from ``copilot --output-format=json`` JSONL output. + + Copilot emits one JSON object per line. The reply is carried by + ``assistant.message`` events; the last one is the final answer. Non-JSON + lines and event types we do not care about are skipped. + + Raises: + AgentCLIError: on empty stdout or when no assistant message is present. + """ + text = raw.strip() + if not text: + raise AgentCLIError("copilot returned empty stdout") + last_text = "" + for line in text.splitlines(): + line = line.strip() + if not line or not line.startswith("{"): + continue + try: + obj: Any = json.loads(line) + except json.JSONDecodeError: + continue + if not isinstance(obj, dict): + continue + if obj.get("type") == "assistant.message": + data = obj.get("data") + if isinstance(data, dict): + content = data.get("content") + if isinstance(content, str) and content.strip(): + last_text = content + elif obj.get("type") == "result" and obj.get("exitCode") not in (0, None): + raise AgentCLIError(f"copilot reported exitCode={obj.get('exitCode')!r}") + if not last_text: + raise AgentCLIError( + f"copilot returned no assistant message in JSONL output; raw={raw[:400]!r}" + ) + return last_text + + +def _copilot_auth_check(binary: str) -> tuple[bool, str | None]: + """Check Copilot has local credentials available (no inference). + + Copilot CLI 1.0.88 exposes no cheap non-interactive auth-status command. + ``copilot login`` resolves credentials from ``COPILOT_GITHUB_TOKEN`` / + ``GH_TOKEN`` / ``GITHUB_TOKEN`` or from the system credential store backed + by ``~/.copilot``, so we probe for those. A real call still fails closed if + the stored session has expired. + """ + if any(os.environ.get(var) for var in ("COPILOT_GITHUB_TOKEN", "GH_TOKEN", "GITHUB_TOKEN")): + return True, None + if os.path.isdir(os.path.join(os.path.expanduser("~"), ".copilot")): + return True, None + return False, "copilot is not authenticated (run `copilot login`)" + + # --------------------------------------------------------------------------- # Antigravity CLI (registered but DISABLED — verified incompatible) # @@ -968,6 +1074,7 @@ class CliSpec: _prepare_opencode_env, _preflight_opencode_policy, ), + "copilot": CliSpec("copilot", _build_copilot_argv, _parse_copilot_output, _copilot_auth_check), # Disabled (fails closed via _build_agy_argv). agy's backend is Gemini, so it # reuses _parse_gemini_output rather than duplicating it — though parse is # never reached while _build_agy_argv raises. See the antigravity note above. diff --git a/src/skillspector/providers/copilot_cli/__init__.py b/src/skillspector/providers/copilot_cli/__init__.py new file mode 100644 index 000000000..5481d7ce5 --- /dev/null +++ b/src/skillspector/providers/copilot_cli/__init__.py @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: Apache-2.0 +"""GitHub Copilot CLI provider — uses the locally-installed ``copilot`` binary. + +No provider API key required. Authentication is managed by the Copilot CLI's +own session (``copilot login``). Set ``SKILLSPECTOR_PROVIDER=copilot_cli`` to +activate. +""" + +from .provider import CopilotCLIProvider + +__all__ = ["CopilotCLIProvider"] diff --git a/src/skillspector/providers/copilot_cli/provider.py b/src/skillspector/providers/copilot_cli/provider.py new file mode 100644 index 000000000..ac512ce28 --- /dev/null +++ b/src/skillspector/providers/copilot_cli/provider.py @@ -0,0 +1,28 @@ +# SPDX-License-Identifier: Apache-2.0 +"""GitHub Copilot CLI provider — Stage-2 LLM analysis via the local ``copilot`` binary. + +Activated by ``SKILLSPECTOR_PROVIDER=copilot_cli``. Authentication is handled by +the Copilot CLI's own session (``copilot login``) or a ``COPILOT_GITHUB_TOKEN`` / +``GH_TOKEN`` / ``GITHUB_TOKEN`` environment token; no provider API key is read. + +All behaviour is inherited from +:class:`skillspector.providers._agent_cli_base.AgentCLIProviderBase`; the +"copilot"-specific argv, output parsing, and auth probe live in the +:mod:`skillspector.providers._agent_cli` registry. +""" + +from __future__ import annotations + +from skillspector.providers._agent_cli_base import AgentCLIProviderBase + +BINARY_NAME = "copilot" + + +class CopilotCLIProvider(AgentCLIProviderBase): + """GitHub Copilot CLI provider (no API key; uses the local ``copilot`` login). + + No model is pinned: ``copilot`` runs with the CLI default model. Set + ``SKILLSPECTOR_MODEL`` to override, e.g. ``claude-sonnet-5``. + """ + + BINARY_NAME = "copilot" diff --git a/tests/unit/test_agent_cli.py b/tests/unit/test_agent_cli.py index 4e1c73cb4..1a4cfc98a 100644 --- a/tests/unit/test_agent_cli.py +++ b/tests/unit/test_agent_cli.py @@ -35,8 +35,10 @@ import io import json +import os import subprocess import sys +from pathlib import Path from unittest.mock import MagicMock, patch import pytest @@ -683,10 +685,17 @@ def test_timeout_returns_none(self) -> None: class TestCliRegistry: def test_registry_covers_known_clis(self) -> None: - assert set(_agent_cli._REGISTRY) == {"claude", "codex", "gemini", "opencode", "agy"} + assert set(_agent_cli._REGISTRY) == { + "claude", + "codex", + "gemini", + "opencode", + "copilot", + "agy", + } def test_get_spec_returns_matching_binary(self) -> None: - for name in ("claude", "codex", "gemini", "opencode", "agy"): + for name in ("claude", "codex", "gemini", "opencode", "copilot", "agy"): assert _agent_cli.get_spec(name).binary == name def test_get_spec_unknown_raises(self) -> None: @@ -745,6 +754,79 @@ def test_parse_handles_multiple_text_keys(self) -> None: assert _agent_cli._parse_gemini_output(json.dumps({key: "answer"})) == "answer" +class TestCopilotArgv: + """Flags verified against GitHub Copilot CLI 1.0.88; the security invariants + (no tool execution, no auto-approve, model validated) must hold.""" + + def test_argv_has_binary_and_model_no_bypass(self) -> None: + argv = _agent_cli._build_copilot_argv("copilot", "claude-sonnet-5", 4096) + assert argv[0] == "copilot" + assert "--model" in argv and "claude-sonnet-5" in argv + full = " ".join(argv) + assert "yolo" not in full + assert "--allow-all" not in full + # Exclusive allowlist naming no real tool -> every tool is stripped. + assert f"--available-tools={_agent_cli._COPILOT_NO_TOOLS_SENTINEL}" in argv + assert "--output-format=json" in argv + + def test_prompt_never_placed_in_argv(self) -> None: + # No -p/--prompt: the prompt is piped to stdin by run_agent_cli. + argv = _agent_cli._build_copilot_argv("copilot", "", 4096) + assert "-p" not in argv and "--prompt" not in argv + + def test_model_label_validated_against_injection(self) -> None: + with pytest.raises(AgentCLIError): + _agent_cli._build_copilot_argv("copilot", "--inject", 4096) + + def test_model_flag_omitted_when_empty(self) -> None: + # No SKILLSPECTOR_MODEL -> copilot runs with the user's own model. + argv = _agent_cli._build_copilot_argv("copilot", "", 4096) + assert "--model" not in argv + + def test_parse_returns_last_assistant_message(self) -> None: + raw = "\n".join( + json.dumps(obj) + for obj in ( + {"type": "session.info", "data": {"message": "noise"}}, + {"type": "assistant.message", "data": {"content": "first"}}, + {"type": "assistant.message", "data": {"content": "final"}}, + {"type": "result", "exitCode": 0}, + ) + ) + assert _agent_cli._parse_copilot_output(raw) == "final" + + def test_parse_skips_non_json_lines(self) -> None: + raw = 'warning: something\n{"type": "assistant.message", "data": {"content": "ok"}}' + assert _agent_cli._parse_copilot_output(raw) == "ok" + + def test_parse_empty_stdout_raises(self) -> None: + with pytest.raises(AgentCLIError, match="empty stdout"): + _agent_cli._parse_copilot_output(" ") + + def test_parse_without_assistant_message_raises(self) -> None: + with pytest.raises(AgentCLIError, match="no assistant message"): + _agent_cli._parse_copilot_output(json.dumps({"type": "session.info", "data": {}})) + + def test_parse_nonzero_result_exit_code_raises(self) -> None: + raw = json.dumps({"type": "result", "exitCode": 1}) + with pytest.raises(AgentCLIError, match="exitCode"): + _agent_cli._parse_copilot_output(raw) + + def test_auth_check_accepts_env_token(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("COPILOT_GITHUB_TOKEN", "ghu_example") + assert _agent_cli._copilot_auth_check("copilot") == (True, None) + + def test_auth_check_fails_closed_without_credentials( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + for var in ("COPILOT_GITHUB_TOKEN", "GH_TOKEN", "GITHUB_TOKEN"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setattr(os.path, "expanduser", lambda _p: str(tmp_path)) + ok, reason = _agent_cli._copilot_auth_check("copilot") + assert ok is False + assert "copilot login" in (reason or "") + + class TestAntigravityDisabled: """`agy` is registered but disabled (TTY-only, uncapturable): fail closed."""