From 2b00db0bc2958ce8988945f0cf8750eec1b351cb Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Mon, 5 Oct 2026 10:43:48 +0530 Subject: [PATCH 1/5] fix(security): allowlist agent CLI environment variables Signed-off-by: yashrajbasav --- src/skillspector/providers/_agent_cli.py | 73 +++++++++++------------- tests/provider/test_opencode_cli.py | 42 ++++++-------- tests/unit/test_agent_cli.py | 44 ++++++++++++-- 3 files changed, 92 insertions(+), 67 deletions(-) diff --git a/src/skillspector/providers/_agent_cli.py b/src/skillspector/providers/_agent_cli.py index 206be2b9f..23f8f3ac6 100644 --- a/src/skillspector/providers/_agent_cli.py +++ b/src/skillspector/providers/_agent_cli.py @@ -75,35 +75,35 @@ 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", - "ANTHROPIC_PROXY_API_KEY", - "OPENAI_API_KEY", - "SKILLSPECTOR_API_KEYS", - "SKILLSPECTOR_COMPAT_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", +# Only runtime and local-login discovery variables cross into agent processes. +# Do not inherit provider keys, arbitrary application variables, loader hooks, +# or CLI configuration overrides from the operator's environment. +_RUNTIME_ENV_NAMES = frozenset( + { + "PATH", + "PATHEXT", + "HOME", + "USERPROFILE", + "APPDATA", + "LOCALAPPDATA", + "SYSTEMROOT", + "WINDIR", + "TEMP", + "TMP", + "TMPDIR", + "XDG_DATA_HOME", + "LANG", + "LANGUAGE", + "LC_ALL", + "LC_CTYPE", + "LC_COLLATE", + "LC_MESSAGES", + "LC_MONETARY", + "LC_NUMERIC", + "LC_TIME", + "TERM", + "NO_COLOR", + } ) @@ -117,18 +117,13 @@ class AgentCLIError(RuntimeError): def _scrub_env() -> dict[str, str]: - """Return a copy of ``os.environ`` with secret variables removed. + """Keep only runtime variables needed by the CLIs and their local logins. - Any variable whose name starts with a prefix in ``_SECRET_ENV_PREFIXES`` - is stripped. The resulting environment is passed to the subprocess. + Exact names (case-insensitive for Windows) fail closed for new secret names + and configuration overrides. Provider API keys are deliberately excluded: + agent CLI providers authenticate through the user's existing local login. """ - 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 + return {key: val for key, val in os.environ.items() if key.upper() in _RUNTIME_ENV_NAMES} # --------------------------------------------------------------------------- diff --git a/tests/provider/test_opencode_cli.py b/tests/provider/test_opencode_cli.py index c253eaf03..52402d394 100644 --- a/tests/provider/test_opencode_cli.py +++ b/tests/provider/test_opencode_cli.py @@ -323,7 +323,13 @@ def test_policy_overrides_hostile_ambient_configuration(self, tmp_path: Path) -> assert Path(env["OPENCODE_TEST_MANAGED_CONFIG_DIR"]).is_relative_to(tmp_path) @staticmethod - def _write_fake_opencode(binary: Path) -> None: + def _write_fake_opencode( + binary: Path, + markers: Path, + *, + managed: dict | None = None, + version: str = _OPENCODE_SUPPORTED_VERSION, + ) -> None: """Write a host simulator with real version/config/run boundaries.""" binary.write_text( textwrap.dedent( @@ -335,11 +341,11 @@ def _write_fake_opencode(binary: Path) -> None: from pathlib import Path if sys.argv[1:] == ["--version"]: - print(os.environ.get("FAKE_OPENCODE_VERSION", {_OPENCODE_SUPPORTED_VERSION!r})) + print({version!r}) raise SystemExit(0) config = json.loads(os.environ["OPENCODE_CONFIG_CONTENT"]) - managed = json.loads(os.environ.get("HOSTILE_MANAGED_CONFIG", "{{}}")) + managed = {managed or {}!r} config.setdefault("agent", {{}}).update(managed.get("agent", {{}})) for key, value in managed.items(): if key != "agent": @@ -356,9 +362,9 @@ def _write_fake_opencode(binary: Path) -> None: adapters = ["bash", "read", "edit", "webfetch", "websearch", "mcp_host", "skill", "future_host_tool"] if not denied: for adapter in adapters: - (Path(os.environ["ATTACK_MARKERS"]) / adapter).write_text("executed") + (Path({str(markers)!r}) / adapter).write_text("executed") if config.get("share") != "disabled" or os.environ.get("OPENCODE_AUTO_SHARE") not in ("0", "false"): - (Path(os.environ["ATTACK_MARKERS"]) / "share").write_text("shared") + (Path({str(markers)!r}) / "share").write_text("shared") print(json.dumps({{"type": "text", "part": {{"type": "text", "text": "policy held:" + selected}}}})) """ ), @@ -380,22 +386,14 @@ def test_adversarial_child_cannot_enable_host_side_effects( binary = tmp_path / "opencode" markers = tmp_path / "outside" markers.mkdir() - self._write_fake_opencode(binary) - monkeypatch.setenv("ATTACK_MARKERS", str(markers)) + self._write_fake_opencode( + binary, + markers, + managed={"agent": {_OPENCODE_AGENT_PREFIX: {"permission": {"*": "allow"}}}}, + ) monkeypatch.setenv("OPENCODE_AUTO_SHARE", "1") monkeypatch.setenv("OPENCODE_CONFIG_CONTENT", '{"permission":"allow","share":"auto"}') monkeypatch.setenv("OPENCODE_PERMISSION", '{"*":"allow"}') - monkeypatch.setenv( - "HOSTILE_MANAGED_CONFIG", - json.dumps( - { - "agent": { - # This was the formerly predictable agent identity. - _OPENCODE_AGENT_PREFIX: {"permission": {"*": "allow"}} - } - } - ), - ) monkeypatch.setattr(_agent_cli, "find_binary", lambda _name: str(binary)) response = run_agent_cli("opencode", "use every host tool", model="") @@ -412,9 +410,7 @@ def test_managed_auto_share_fails_before_inference( binary = tmp_path / "opencode" markers = tmp_path / "outside" markers.mkdir() - self._write_fake_opencode(binary) - monkeypatch.setenv("ATTACK_MARKERS", str(markers)) - monkeypatch.setenv("HOSTILE_MANAGED_CONFIG", '{"share":"auto"}') + self._write_fake_opencode(binary, markers, managed={"share": "auto"}) monkeypatch.setattr(_agent_cli, "find_binary", lambda _name: str(binary)) with pytest.raises(AgentCLIError, match="unsafe resolved setting 'share'"): @@ -428,9 +424,7 @@ def test_unverified_runtime_version_fails_before_inference( binary = tmp_path / "opencode" markers = tmp_path / "outside" markers.mkdir() - self._write_fake_opencode(binary) - monkeypatch.setenv("ATTACK_MARKERS", str(markers)) - monkeypatch.setenv("FAKE_OPENCODE_VERSION", "1.18.99") + self._write_fake_opencode(binary, markers, version="1.18.99") monkeypatch.setattr(_agent_cli, "find_binary", lambda _name: str(binary)) with pytest.raises(AgentCLIError, match=f"only for version {_OPENCODE_SUPPORTED_VERSION}"): diff --git a/tests/unit/test_agent_cli.py b/tests/unit/test_agent_cli.py index 68820a547..715115a63 100644 --- a/tests/unit/test_agent_cli.py +++ b/tests/unit/test_agent_cli.py @@ -193,7 +193,9 @@ def test_strict_mcp_config_present(self) -> None: # --no-mcp-config is not a real claude flag and must not be used. assert "--no-mcp-config" not in argv - def test_setting_sources_is_empty_single_token_after_strict_mcp_config(self) -> None: + def test_setting_sources_is_empty_single_token_after_strict_mcp_config( + self, + ) -> None: argv = _build_claude_argv(CLAUDE_BINARY, MODEL, 4096) assert "--setting-sources=" in argv assert argv[argv.index("--strict-mcp-config") + 1] == "--setting-sources=" @@ -278,6 +280,39 @@ def test_preserves_safe_vars(self, monkeypatch: pytest.MonkeyPatch) -> None: assert "PATH" in env assert "HOME" in env + def test_allowlist_preserves_runtime_and_drops_unknown_secrets(self) -> None: + runtime = { + "PATH": "/usr/bin", + "HOME": "/home/operator", + "LANG": "C.UTF-8", + "LC_ALL": "C.UTF-8", + "TERM": "dumb", + "TMPDIR": "/tmp", + "SystemRoot": "C:/Windows", + "AppData": "C:/Users/operator/AppData", + "XDG_DATA_HOME": "/home/operator/.local/share", + } + secrets = dict.fromkeys( + ( + "ANTHROPIC_AUTH_TOKEN", + "GEMINI_API_KEY", + "OPENROUTER_API_KEY", + "NPM_TOKEN", + "PYPI_TOKEN", + "DATABASE_URL", + "VAULT_TOKEN", + "future_vendor_auth", + "LC_SECRET", + "NODE_OPTIONS", + "PYTHONPATH", + ), + "synthetic-secret", + ) + original = runtime | secrets + with patch.dict(_agent_cli.os.environ, original, clear=True): + assert _scrub_env() == runtime + assert dict(_agent_cli.os.environ) == original + @pytest.mark.parametrize("binary_name", ["claude", "gemini"]) @pytest.mark.parametrize("lowercase", [False, True]) def test_project_credentials_never_reach_child( @@ -297,7 +332,8 @@ def test_project_credentials_never_reach_child( output = _GOOD_CLAUDE_OUTPUT with ( patch( - "skillspector.providers._agent_cli.find_binary", return_value="/usr/bin/mock-cli" + "skillspector.providers._agent_cli.find_binary", + return_value="/usr/bin/mock-cli", ), patch("skillspector.providers._agent_cli.subprocess.Popen") as popen, ): @@ -305,8 +341,8 @@ def test_project_credentials_never_reach_child( run_agent_cli(binary_name, PROMPT, model="") child_env = popen.call_args.kwargs["env"] assert not set(secrets).intersection(child_env) - assert child_env["SKILLSPECTOR_MODEL"] == "test-model" - assert child_env["SKILLSPECTOR_COMPAT_BASE_URL"] == "https://example.invalid" + assert "SKILLSPECTOR_MODEL" not in child_env + assert "SKILLSPECTOR_COMPAT_BASE_URL" not in child_env for key in secrets: assert _agent_cli.os.environ[key] == "synthetic-credential" From 349b6921ee249164ffe07cfd09708f35bd32d11f Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Mon, 5 Oct 2026 21:21:06 +0530 Subject: [PATCH 2/5] docs: mark the Codex CLI provider disabled Signed-off-by: yashrajbasav --- .env.example | 5 +++-- README.md | 13 +++++-------- docs/DEVELOPMENT.md | 14 ++++++++------ src/skillspector/cli.py | 7 ++++--- src/skillspector/providers/__init__.py | 2 +- src/skillspector/providers/_agent_cli.py | 6 ++++-- src/skillspector/providers/codex_cli/__init__.py | 12 ++++-------- tests/integration/test_agent_cli_live.py | 9 ++++++--- 8 files changed, 35 insertions(+), 33 deletions(-) diff --git a/.env.example b/.env.example index 581461142..fb1f7f71f 100644 --- a/.env.example +++ b/.env.example @@ -4,7 +4,8 @@ ENV=dev # options: dev|s # metadata, and default-model lookups. Leave unset to default to nv_build. # Options: openai | anthropic | anthropic_proxy | bedrock | nv_build | # ollama | azure_openai | openai_compatible | claude_cli | -# codex_cli | gemini_cli | opencode_cli +# gemini_cli | opencode_cli +# codex_cli is registered but disabled: its read-only sandbox permits host-file reads. SKILLSPECTOR_PROVIDER= # Aggregate deadline for one complete scan workflow. Defaults to 600 seconds; @@ -67,7 +68,7 @@ AZURE_OPENAI_ENDPOINT= SKILLSPECTOR_COMPAT_API_KEY= SKILLSPECTOR_COMPAT_BASE_URL= -# claude_cli, codex_cli, gemini_cli, and opencode_cli use their CLI's existing local +# claude_cli, gemini_cli, and opencode_cli use their CLI's existing local # authentication session and do not need an API key here. # SkillSpector config diff --git a/README.md b/README.md index 130039008..5a6776e4f 100644 --- a/README.md +++ b/README.md @@ -263,7 +263,7 @@ inference gateways. | `azure_openai` | `AZURE_OPENAI_API_KEY` + `AZURE_OPENAI_ENDPOINT` | Azure OpenAI Service | `gpt-4o` (deployment defaults to the model label) | | `openai_compatible` | `SKILLSPECTOR_COMPAT_API_KEY` + `SKILLSPECTOR_COMPAT_BASE_URL` | Any OpenAI-compatible endpoint | `llama-3.1-70b-versatile` | | `claude_cli` | _(none — uses local CLI auth)_ | local `claude` binary | local Claude runtime fallback, or `SKILLSPECTOR_MODEL` | -| `codex_cli` | _(none — uses local CLI auth)_ | local `codex` binary | local Codex runtime fallback, or `SKILLSPECTOR_MODEL` | +| `codex_cli` | Disabled | Registered for compatibility; its read-only sandbox permits host-file reads | Use an HTTP API provider or another supported CLI provider | | `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.33 binary | local OpenCode runtime fallback, or `SKILLSPECTOR_MODEL` | @@ -327,11 +327,6 @@ export SKILLSPECTOR_PROVIDER=claude_cli # export SKILLSPECTOR_MODEL=claude-sonnet-4-6 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/ - # Gemini (via OpenAI compatibility layer) export SKILLSPECTOR_PROVIDER=openai export OPENAI_API_KEY="YOUR_GEMINI_API_KEY" @@ -647,7 +642,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`, `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 | | `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 | @@ -675,7 +670,9 @@ 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. +> **Disabled provider:** `codex_cli` remains registered but cannot run LLM analysis because its read-only sandbox allows host-file reads. Existing users should select an HTTP API provider or another supported CLI provider. + +> **CLI providers** (`claude_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. > > `opencode_cli` currently fails closed unless the installed OpenCode version is exactly `1.18.33`, the version whose configuration precedence and deny-all semantics are verified by this release. diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 136c432b5..000402831 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -34,7 +34,7 @@ make install-dev - **Python**: 3.12+ (see [pyproject.toml](../pyproject.toml)). `make install` and `make install-dev` use **uv** if available (`uv sync` / `uv sync --all-extras`), otherwise **pip** (`pip install -e .` / `pip install -e ".[dev]"`). You must create and activate the virtual environment yourself before running any make target. - **Environment**: Optional `.env` in the project root. The LangGraph dev server loads it (see [langgraph.json](../langgraph.json) `"env": ".env"`). Key variables: - - **`SKILLSPECTOR_PROVIDER`**: Selects the active LLM provider — `openai`, `anthropic`, `anthropic_proxy`, `bedrock`, `nv_build`, `ollama`, `azure_openai`, `openai_compatible`, `claude_cli`, `codex_cli`, `gemini_cli`, or `opencode_cli`. Defaults to `nv_build` when unset. + - **`SKILLSPECTOR_PROVIDER`**: Selects the active LLM provider — `openai`, `anthropic`, `anthropic_proxy`, `bedrock`, `nv_build`, `ollama`, `azure_openai`, `openai_compatible`, `claude_cli`, `gemini_cli`, or `opencode_cli`. Defaults to `nv_build` when unset. - **Provider credential**: depends on the active provider. Hosted providers use the matching variables in [.env.example](../.env.example); Ollama and CLI providers do not require an API key. See [providers/](../src/skillspector/providers/). - **`OPENAI_BASE_URL`**: Override the OpenAI endpoint (e.g. point at Ollama). - **`SKILLSPECTOR_MODEL`**: Override default model; see [constants.py](../src/skillspector/constants.py). @@ -296,7 +296,7 @@ Copy [.env.example](../.env.example) to `.env` in the project root and set value | Variable | Description | Example | |----------|-------------|---------| -| `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`. Defaults to `nv_build`. | `claude_cli` | +| `SKILLSPECTOR_PROVIDER` | Active LLM provider: `openai` \| `anthropic` \| `anthropic_proxy` \| `bedrock` \| `nv_build` \| `ollama` \| `azure_openai` \| `openai_compatible` \| `claude_cli` \| `gemini_cli` \| `opencode_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` | @@ -312,7 +312,9 @@ Copy [.env.example](../.env.example) to `.env` in the project root and set value | `SKILLSPECTOR_COMPAT_BASE_URL` | OpenAI-compatible endpoint base URL. | `https://api.groq.com/openai/v1` | | `SKILLSPECTOR_MODEL` | Override the active provider's bundled default model (see [README.md](../README.md) for per-provider defaults). CLI providers forward it as `--model`. | `gpt-5.2` | -> **CLI providers** (`claude_cli`, `codex_cli`, `gemini_cli`, `opencode_cli`): no credential env var is needed. Authentication is managed by the agent CLI's own session. The subprocess is heavily sandboxed — see [providers/_agent_cli.py](../src/skillspector/providers/_agent_cli.py). +> **Disabled provider:** `codex_cli` remains registered for compatibility but refuses inference until a complete no-tools policy is verified. + +> **CLI providers** (`claude_cli`, `gemini_cli`, `opencode_cli`): no credential env var is needed. Authentication is managed by the agent CLI's own session. The subprocess is heavily sandboxed — see [providers/_agent_cli.py](../src/skillspector/providers/_agent_cli.py). ### Live provider tests @@ -343,15 +345,15 @@ Base URL env vars are not needed for live provider tests; the tests intentionall - `azure_openai/` — Azure OpenAI Service (`AZURE_OPENAI_API_KEY`, `AZURE_OPENAI_ENDPOINT`) - `openai_compatible/` — generic compatible endpoint (`SKILLSPECTOR_COMPAT_API_KEY`, `SKILLSPECTOR_COMPAT_BASE_URL`) - `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`. + - `codex_cli/` — **registered but disabled**. Its read-only sandbox permits host-file reads. Select an HTTP API provider or another supported CLI provider for LLM analysis. - `gemini_cli/` — **local `gemini` binary; no API key**. Uses the CLI's own auth session. Set `SKILLSPECTOR_PROVIDER=gemini_cli`. - `opencode_cli/` — **local `opencode` 1.18.33 binary; no API key**. Uses the CLI's own auth session (`opencode auth login`) and fails closed on every other runtime version because the deny-all policy is verified against that exact release. Set `SKILLSPECTOR_PROVIDER=opencode_cli`. - CLI providers (`claude_cli`, `codex_cli`, `gemini_cli`, `opencode_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. + CLI providers (`claude_cli`, `gemini_cli`, `opencode_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. + - **CLI providers** (`claude_cli`, `gemini_cli`, `opencode_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/cli.py b/src/skillspector/cli.py index 1e3d43fae..f92cfc900 100644 --- a/src/skillspector/cli.py +++ b/src/skillspector/cli.py @@ -612,7 +612,7 @@ def scan( SKILLSPECTOR_PROVIDER Active LLM provider: openai | anthropic | anthropic_proxy | bedrock | nv_build | nv_inference | ollama | azure_openai | - openai_compatible | claude_cli | codex_cli | + openai_compatible | claude_cli | gemini_cli | opencode_cli. Defaults to the NVIDIA path (nv_inference, falling back to nv_build in OSS builds). @@ -635,9 +635,10 @@ def scan( SKILLSPECTOR_COMPAT_API_KEY + SKILLSPECTOR_COMPAT_BASE_URL for openai_compatible - ollama uses the local Ollama service. claude_cli, codex_cli, + ollama uses the local Ollama service. claude_cli, gemini_cli, and opencode_cli use their CLI's existing local - authentication session. + authentication session. codex_cli is registered but disabled because + its read-only sandbox permits host-file reads; use another provider. """ if exclude and ( recursive diff --git a/src/skillspector/providers/__init__.py b/src/skillspector/providers/__init__.py index 754fc2c5a..23287a9c3 100644 --- a/src/skillspector/providers/__init__.py +++ b/src/skillspector/providers/__init__.py @@ -31,7 +31,7 @@ azure_openai → AzureOpenAIProvider (Azure OpenAI Service) openai_compatible → OpenAICompatibleProvider (Groq, Together AI, Mistral, etc.) claude_cli → ClaudeCLIProvider (local ``claude`` binary, no API key) - codex_cli → CodexCLIProvider (local ``codex`` binary, no API key) + codex_cli → CodexCLIProvider (registered but disabled: host-file reads) gemini_cli → GeminiCLIProvider (local ``gemini`` binary, no API key) opencode_cli → OpencodeCLIProvider (local ``opencode`` binary, no API key) antigravity_cli → AntigravityCLIProvider (local ``agy`` binary; registered diff --git a/src/skillspector/providers/_agent_cli.py b/src/skillspector/providers/_agent_cli.py index 23f8f3ac6..0b6df63ad 100644 --- a/src/skillspector/providers/_agent_cli.py +++ b/src/skillspector/providers/_agent_cli.py @@ -41,8 +41,8 @@ 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. +and the assistant text is returned. The Codex JSONL parser is retained for +compatibility, but Codex inference is disabled before a subprocess is started. """ from __future__ import annotations @@ -271,6 +271,8 @@ def _parse_claude_output(raw: str) -> str: # --------------------------------------------------------------------------- +# Registered but disabled. Re-enabling requires a pinned CLI version and a +# verified policy preflight that denies every model-driven tool and host-file read. _CODEX_DISABLED_REASON = ( "codex_cli is disabled: its read-only sandbox still permits commands to read " "host files, and SkillSpector has no verified deny-all-tools policy for Codex. " diff --git a/src/skillspector/providers/codex_cli/__init__.py b/src/skillspector/providers/codex_cli/__init__.py index f5f60c1aa..6495d702c 100644 --- a/src/skillspector/providers/codex_cli/__init__.py +++ b/src/skillspector/providers/codex_cli/__init__.py @@ -13,15 +13,11 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Codex CLI provider — uses the locally-installed ``codex`` binary. +"""Codex CLI provider — registered but disabled. -No API key required. Authentication is managed by the ``codex`` CLI's -own session (``codex login``). Set ``SKILLSPECTOR_PROVIDER=codex_cli`` -to activate. - -NOTE: codex_cli support is implemented using the same hardened subprocess -helper as claude_cli (``_agent_cli.run_agent_cli``). See provider.py for -sandbox flags and limitations. +The read-only sandbox still permits model-driven host-file reads. Availability +checks and inference therefore fail closed without starting Codex. Use an HTTP +API provider or another supported CLI provider for LLM analysis. """ from .provider import CodexCLIProvider diff --git a/tests/integration/test_agent_cli_live.py b/tests/integration/test_agent_cli_live.py index 8bf7ba533..976e773e9 100644 --- a/tests/integration/test_agent_cli_live.py +++ b/tests/integration/test_agent_cli_live.py @@ -17,7 +17,7 @@ WHY THESE ARE OPTIONAL ---------------------- -These tests invoke the REAL local agent CLIs (``claude`` / ``codex`` / +These tests invoke the REAL local agent CLIs (``claude`` / ``gemini``), so they are marked ``integration`` and are therefore EXCLUDED from the default test run — ``pyproject.toml`` sets ``addopts = -m 'not integration'``. A developer (at NVIDIA or anywhere) who does **not** have any of @@ -28,7 +28,7 @@ When you DO opt in with ``-m integration``, each case additionally SKIPS per-CLI when that binary is absent or unauthenticated. So if you only have -``codex`` installed, the codex cases run and the claude/gemini cases skip +``claude`` installed, the claude cases run and the gemini cases skip cleanly — a missing tool never fails the suite. # exercise whichever agent CLIs you happen to have installed + logged in: @@ -41,6 +41,9 @@ executed (the capability-stripped, fail-closed invocation; the flags that guarantee this are unit-tested in ``tests/unit/test_agent_cli.py``). +``codex`` is excluded because its read-only sandbox permits host-file reads; +its fail-closed behaviour is covered by unit tests. + ``agy`` (Antigravity) is intentionally NOT covered: it is registered but DISABLED — its print mode renders to a TTY and returns nothing on a pipe, so it cannot be driven programmatically (see the note in @@ -58,7 +61,7 @@ #: Real ``stdin -> stdout`` agent CLIs this harness can drive. Add a new CLI here #: once it is wired into the ``_agent_cli`` registry and verified end-to-end. -LIVE_CLIS = ["claude", "codex", "gemini"] +LIVE_CLIS = ["claude", "gemini"] def _require(cli: str) -> None: From a8a816d44ea2078bc56b5f911339f8a59c7e236e Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Mon, 5 Oct 2026 21:21:58 +0530 Subject: [PATCH 3/5] fix: preserve network settings in scrubbed CLI environments Signed-off-by: yashrajbasav --- src/skillspector/providers/_agent_cli.py | 16 +++++++++-- tests/unit/test_agent_cli.py | 35 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/skillspector/providers/_agent_cli.py b/src/skillspector/providers/_agent_cli.py index 0b6df63ad..18d2fc805 100644 --- a/src/skillspector/providers/_agent_cli.py +++ b/src/skillspector/providers/_agent_cli.py @@ -75,7 +75,7 @@ MAX_STDERR_BYTES = 64_000 # stderr is only used for error snippets CLI_TIMEOUT_SECONDS = 300 # 5-minute per-call hard limit -# Only runtime and local-login discovery variables cross into agent processes. +# Only runtime, network transport, and local-login discovery variables cross into agent processes. # Do not inherit provider keys, arbitrary application variables, loader hooks, # or CLI configuration overrides from the operator's environment. _RUNTIME_ENV_NAMES = frozenset( @@ -92,6 +92,14 @@ "TMP", "TMPDIR", "XDG_DATA_HOME", + "CLAUDE_CONFIG_DIR", + "HTTP_PROXY", + "HTTPS_PROXY", + "ALL_PROXY", + "NO_PROXY", + "NODE_EXTRA_CA_CERTS", + "SSL_CERT_FILE", + "SSL_CERT_DIR", "LANG", "LANGUAGE", "LC_ALL", @@ -737,7 +745,11 @@ def _claude_auth_check(binary: str) -> tuple[bool, str | None]: """Check claude is authenticated via ``claude auth status`` (no inference).""" try: result = subprocess.run( - [binary, "auth", "status"], capture_output=True, shell=False, timeout=15 + [binary, "auth", "status"], + capture_output=True, + shell=False, + timeout=15, + env=_scrub_env(), ) except (subprocess.TimeoutExpired, FileNotFoundError, OSError) as exc: return False, f"claude auth status check failed: {exc}" diff --git a/tests/unit/test_agent_cli.py b/tests/unit/test_agent_cli.py index 715115a63..2e9c575b4 100644 --- a/tests/unit/test_agent_cli.py +++ b/tests/unit/test_agent_cli.py @@ -291,11 +291,28 @@ def test_allowlist_preserves_runtime_and_drops_unknown_secrets(self) -> None: "SystemRoot": "C:/Windows", "AppData": "C:/Users/operator/AppData", "XDG_DATA_HOME": "/home/operator/.local/share", + "CLAUDE_CONFIG_DIR": "/home/operator/claude-login", + "HTTPS_PROXY": "http://proxy.example:8080", + "https_proxy": "http://proxy.example:8080", + "HTTP_PROXY": "http://proxy.example:8080", + "http_proxy": "http://proxy.example:8080", + "ALL_PROXY": "socks5://proxy.example:1080", + "all_proxy": "socks5://proxy.example:1080", + "NO_PROXY": "localhost", + "no_proxy": "localhost", + "NODE_EXTRA_CA_CERTS": "/certs/ca.pem", + "SSL_CERT_FILE": "/certs/ca.pem", + "SSL_CERT_DIR": "/certs", } secrets = dict.fromkeys( ( "ANTHROPIC_AUTH_TOKEN", "GEMINI_API_KEY", + "GH_TOKEN", + "GH_ENTERPRISE_TOKEN", + "GIT_SSL_CERT", + "GIT_SSL_KEY", + "SSLKEYLOGFILE", "OPENROUTER_API_KEY", "NPM_TOKEN", "PYPI_TOKEN", @@ -313,6 +330,24 @@ def test_allowlist_preserves_runtime_and_drops_unknown_secrets(self) -> None: assert _scrub_env() == runtime assert dict(_agent_cli.os.environ) == original + def test_claude_auth_probe_uses_the_inference_environment(self) -> None: + runtime = { + "PATH": "/usr/bin", + "CLAUDE_CONFIG_DIR": "/home/operator/claude-login", + "HTTPS_PROXY": "http://proxy.example:8080", + } + with ( + patch.dict(_agent_cli.os.environ, runtime | {"GH_TOKEN": "synthetic-secret"}, clear=True), + patch.object(_agent_cli.subprocess, "run") as run, + ): + run.return_value = subprocess.CompletedProcess( + [CLAUDE_BINARY, "auth", "status"], 0, b'{"loggedIn": true}' + ) + assert _agent_cli._claude_auth_check(CLAUDE_BINARY) == (True, None) + assert run.call_args.kwargs["env"] == runtime + assert run.call_args.kwargs["shell"] is False + assert run.call_args.kwargs["timeout"] == 15 + @pytest.mark.parametrize("binary_name", ["claude", "gemini"]) @pytest.mark.parametrize("lowercase", [False, True]) def test_project_credentials_never_reach_child( From 8b9a82c4a20b8db6b7e98fe4a6647e57ee6bf8c1 Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Mon, 5 Oct 2026 21:26:41 +0530 Subject: [PATCH 4/5] style: format review regression coverage Signed-off-by: yashrajbasav --- tests/unit/test_agent_cli.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_agent_cli.py b/tests/unit/test_agent_cli.py index 2e9c575b4..e7b8b6692 100644 --- a/tests/unit/test_agent_cli.py +++ b/tests/unit/test_agent_cli.py @@ -337,7 +337,9 @@ def test_claude_auth_probe_uses_the_inference_environment(self) -> None: "HTTPS_PROXY": "http://proxy.example:8080", } with ( - patch.dict(_agent_cli.os.environ, runtime | {"GH_TOKEN": "synthetic-secret"}, clear=True), + patch.dict( + _agent_cli.os.environ, runtime | {"GH_TOKEN": "synthetic-secret"}, clear=True + ), patch.object(_agent_cli.subprocess, "run") as run, ): run.return_value = subprocess.CompletedProcess( From 37db309d86f9c01419f8401d6259ffb308ce4581 Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Tue, 6 Oct 2026 12:40:10 +0530 Subject: [PATCH 5/5] fix: normalize Windows environment checks and retain account names Signed-off-by: yashrajbasav --- src/skillspector/providers/_agent_cli.py | 3 +++ tests/unit/test_agent_cli.py | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/src/skillspector/providers/_agent_cli.py b/src/skillspector/providers/_agent_cli.py index 944f6687e..a2f9e02b0 100644 --- a/src/skillspector/providers/_agent_cli.py +++ b/src/skillspector/providers/_agent_cli.py @@ -83,6 +83,9 @@ "PATH", "PATHEXT", "HOME", + "USER", + "LOGNAME", + "USERNAME", "USERPROFILE", "APPDATA", "LOCALAPPDATA", diff --git a/tests/unit/test_agent_cli.py b/tests/unit/test_agent_cli.py index e7b8b6692..f371c28fc 100644 --- a/tests/unit/test_agent_cli.py +++ b/tests/unit/test_agent_cli.py @@ -284,6 +284,9 @@ def test_allowlist_preserves_runtime_and_drops_unknown_secrets(self) -> None: runtime = { "PATH": "/usr/bin", "HOME": "/home/operator", + "USER": "operator", + "LOGNAME": "operator", + "USERNAME": "operator", "LANG": "C.UTF-8", "LC_ALL": "C.UTF-8", "TERM": "dumb", @@ -325,6 +328,10 @@ def test_allowlist_preserves_runtime_and_drops_unknown_secrets(self) -> None: ), "synthetic-secret", ) + if sys.platform == "win32": + # Windows os.environ uppercases names and collapses case aliases. + runtime = {key.upper(): value for key, value in runtime.items()} + secrets = {key.upper(): value for key, value in secrets.items()} original = runtime | secrets with patch.dict(_agent_cli.os.environ, original, clear=True): assert _scrub_env() == runtime