Skip to content

feat(providers): add copilot_cli semantic-scan provider - #572

Open
Yoseph-Zuskin wants to merge 5 commits into
NVIDIA:mainfrom
Yoseph-Zuskin:feat/copilot-cli-provider
Open

Yoseph-Zuskin wants to merge 5 commits into
NVIDIA:mainfrom
Yoseph-Zuskin:feat/copilot-cli-provider

Conversation

@Yoseph-Zuskin

@Yoseph-Zuskin Yoseph-Zuskin commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Add copilot_cli semantic-scan provider

Problem

SkillSpector ships CLI providers for Claude, Codex, Gemini, and OpenCode,
but none for GitHub Copilot, so Copilot users get static-only scans
(llm_available stays false and the semantic analyzers are skipped) unless
they provide an OpenAI or Anthropic API key.

Fixes: #8

Approach

  • New copilot_cli provider mirroring the merged opencode_cli shape:
    providers/copilot_cli/{provider.py,__init__.py}, registry entry,
    SKILLSPECTOR_PROVIDER=copilot_cli selection, provider_name() label,
    CLI help text.
  • Transport is flags + piped stdin: copilot -s --no-ask-user with no -p
    flag — the prompt is piped to stdin by run_agent_cli (verified by nonce
    round-trip; -p "" is rejected, so stdin is the cleaner path).
    Untrusted content never reaches argv; list form throughout, shell never
    invoked. Windows hostile-prompt roundtrips covered by test.
  • Least privilege from day one (not a caveat): --available-tools
    names a fixed implausible tool so the model is offered nothing usable
    (verified live: file-creation refused, no side effect), plus
    --deny-tool shell,write belt-and-braces in the documented
    Kind(argument) form (deny wins over allow). Never --allow-all*.
  • --model <label> validated and forwarded only when SKILLSPECTOR_MODEL
    is set; max_output_tokens accepted for CliSpec uniformity and ignored.
  • Auth probe is copilot --version with a 15s timeout, scrubbed env,
    fail-closed: non-zero exit, unparseable output, or anything but the
    pinned 1.0.86 fails. No status subcommand exists; authentication works
    via login session or one of COPILOT_GITHUB_TOKEN/GH_TOKEN/
    GITHUB_TOKEN (preserved deliberately by _prepare_copilot_env, which
    drops every other COPILOT_* and forces COPILOT_AUTO_UPDATE=false).
  • No per-call preflight (no per-invocation policy to verify, unlike
    opencode's config layers) and no model registry (CLI fallback, per the
    registry-split lesson).
  • Deliberate exception worth flagging: _prepare_copilot_env preserves
    COPILOT_GITHUB_TOKEN / GH_TOKEN / GITHUB_TOKEN through the scrub
    (everything else COPILOT_* is dropped). Rationale: these are the CLI's
    documented headless auth path — without them, token-only CI setups (where
    GITHUB_TOKEN is often the only credential) cannot use the provider at
    all, and the issue being resolved is precisely keyless operation. The
    model itself gets no tools, so it cannot read the environment; the CLI
    redacts these variables from its own output by default. If reviewers
    prefer login-only, the fallback is a one-line change (extend the scrub,
    document fail-closed for token setups).
  • Docs trio updated in the same PR (README provider table, .env.example,
    docs/DEVELOPMENT.md); provider tests live in tests/provider/
    per repo convention.

Verification

  • tests/provider/test_copilot_cli.py: 41 passed, 2 POSIX-skipped
    (TDD: argv, auth, parser, wiring, registry label, adversarial).
  • tests/unit/test_agent_cli.py registry loop extended; adversarial
    fake-host checks (deny posture + hostile env) verified via a Windows
    stand-in run: policy held, zero markers.
  • Provider-adjacent suites (test_providers, test_new_providers,
    test_constants, test_llm_utils): 375 passed / 14 skipped total.
  • ruff check + ruff format --check: clean on all touched files.
  • Live single-skill probe (Copilot CLI 1.0.86, Copilot Free, CLI-default
    model): llm_available: true, 3/3 semantic calls succeeded,
    risk 0/LOW, 0 findings.

Sample

Probe scan of one small skill returned
risk: score 0 / LOW / SAFE, llm_calls_attempted: 3,
llm_calls_succeeded: 3, zero findings.

Risks

  • Copilot Free budgets agent requests monthly; bulk scans will exhaust
    them — probes only, no bulk scanning on Free.
  • --available-tools fixed-name posture depends on unknown names staying
    inert (verified on 1.0.86 behavior: silent acceptance); the version
    gate pins exactly 1.0.86 so any CLI behavior change fails closed first.
    The exact pin is deliberate (matches the merged opencode_cli policy):
    the tool-deny behavior was verified against this release, and a silent
    sandbox change must never pass unnoticed. Re-verification per Copilot
    release is the known maintenance cost.
  • DCO: all commits carry Signed-off-by (maintainer: verify on push).

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Reviewed exact head 6bb8dc5a6af1f8e59d4967f2e8133fd00d02bfa6.

The provider wiring, exact-version gate, stdin transport, output bounds, environment filtering, and zero-tool allowlist are well covered. One trust-boundary gap remains: the invocation leaves Copilot CLI's normal custom-instruction discovery enabled while deliberately preserving the user's home/login context. Ambient global or user instructions can therefore alter semantic-security judgments even though COPILOT_CUSTOM_INSTRUCTIONS_DIRS is stripped. Disable custom instructions explicitly for this provider and add an end-to-end argv/isolation regression before enabling it.

Comment thread src/skillspector/providers/_agent_cli.py
- Add providers/copilot_cli/provider.py and __init__.py mirroring
  opencode_cli; register copilot in _agent_cli.py CliSpec with
  _prepare_copilot_env wired in
- Transport is flags + piped stdin: copilot -s --no-ask-user, prompt via
  stdin (verified by nonce round-trip); --available-tools names a fixed
  implausible tool (verified live: model left tool-less, no side effect)
  plus --deny-tool shell,write belt-and-braces; never --allow-all*
- Auth probe is copilot --version (must equal pinned 1.0.85), scrubbed
  env, fail-closed; login session or COPILOT_GITHUB_TOKEN/GH_TOKEN/
  GITHUB_TOKEN auth, everything else COPILOT_* stripped
- Update docs trio: README provider table (+1.0.85 pin note),
  .env.example, docs/DEVELOPMENT.md
- Add tests/provider/test_copilot_cli.py (argv, auth, parser, wiring,
  adversarial fake-host with POSIX-skip) + registry coverage
- Verified: live probe on Copilot Free (CLI-default model),
  llm_available=true, 3/3 calls, 0/LOW, 0 findings; 375 passed /
  14 skipped; ruff + format + diff-check clean

Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com>
Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai>
@Yoseph-Zuskin
Yoseph-Zuskin force-pushed the feat/copilot-cli-provider branch from 6bb8dc5 to 9f8f9c6 Compare September 19, 2026 00:20
Yoseph-Zuskin and others added 2 commits September 18, 2026 20:48
…to-update

- Add --no-custom-instructions to copilot argv so ambient AGENTS.md and
  related files cannot steer the semantic verdict (COPILOT_HOME stays
  for login, hence flag-level disabling)
- Add --disable-builtin-mcps as defense in depth alongside the tool
  allowlist, and --no-auto-update so the version pin cannot invalidate
  mid-scan
- Extend exact-shape, flag-presence, and fake-host adversarial tests;
  document the flags in the builder docstring
- Verified: 134 passed / 2 skipped, ruff clean; live regression with a
  hostile AGENTS.md fixture returns the exact requested reply

Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com>
Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai>
- Bump copilot pin 1.0.85 → 1.0.86 plus all version references (code
  comments, test fixtures, README pin note, DEVELOPMENT); all six
  sandbox flags still present in --help, no policy changes
- Re-ran live AGENTS.md-ignored regression on the new release: PASS
- Verified: file suite 41 passed / 2 skipped; ruff clean

Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com>
Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai>

@yashrajp22 yashrajp22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues remain in the Copilot safety boundary: inference bypasses the exact-version check, and ambient user/plugin lifecycle hooks remain enabled.

Validation: fresh base/head wheels with verified package identity; selected tests passed (324 base, 367 head; 10 skipped each); all 48 no-LLM corpus runs matched across source/wheel and base/head, with nine samples retaining partial reports. Synthetic unsupported-version checks reproduced prompt delivery through both public scans and direct completion. The real 1.0.86 binary reported missing authentication as expected, and the scan retained two PE3 findings while explicitly marking semantic analysis failed.

The hook finding is based on the pinned runtime source. The credential-free marker check was inconclusive; authenticated inference and native hook/tool/MCP behavior remain unverified. Corpus parity is not a global accuracy claim.

Comment on lines +1143 to +1149
"copilot": CliSpec(
"copilot",
_build_copilot_argv,
_parse_copilot_output,
_copilot_auth_check,
_prepare_copilot_env,
),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we run the exact-version check as a completion preflight here? Normal scans and direct complete() calls bypass auth_check. With a synthetic binary reporting 9.9.99, both source and installed-wheel scans sent all three semantic prompts before calling --version; direct completion never called it. That lets an unsupported runtime receive scan content despite the stated 1.0.86 policy. Please reject it before passing stdin and cover the public scan path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — _preflight_copilot_policy is now the copilot CliSpec preflight, so [binary, --version] runs under the isolated child env on every completion and anything but exactly 1.0.86 raises before stdin is written. This covers both paths you named, since public scans and direct complete() both funnel through run_agent_cli. Cost is one local no-inference subprocess per call. Tests: synthetic 9.9.99 is rejected with stdin-never-delivered asserted, plus nonzero/timeout/child-env cases.

Comment on lines +915 to +919
env = {key: value for key, value in base_env.items() if not key.upper().startswith("COPILOT_")}
for name in ("COPILOT_GITHUB_TOKEN", "GH_TOKEN", "GITHUB_TOKEN", "COPILOT_HOME"):
value = os.environ.get(name, "").strip()
if value:
env[name] = value

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we isolate Copilot settings and disable user/plugin hooks here? In the pinned 1.0.86 source, session creation loads user and installed-plugin hooks through the retained HOME/COPILOT_HOME. The tool allowlist and deny rules govern model tools; they do not disable lifecycle command hooks, which can run with scanner privileges and receive prompt content. A temporary working directory and --no-custom-instructions do not close this path. Please preserve authentication separately and cover this with a real harmless-hook check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, with one deviation from the suggested shape that I'd like to be transparent about. I first implemented exactly what you asked — HOME/USERPROFILE/COPILOT_HOME redirected to per-invocation temp dirs — then probed it live and had to revert: 1.0.86 silently refuses inference (exit 0, empty output, nothing created) under any redirected home, including a byte-identical copy and a short-path alias of the real dir, while the same calls succeed with homes untouched. I could not find a redirect the CLI accepts, so isolation would brick the provider rather than harden it.

What landed instead is enforcement by absence: the new _preflight_copilot_policy (already added for the version gate) now also audits the resolved copilot home and raises fail-closed when installed-plugins/ is present and non-empty. Plugins are the documented hook vector (copilot plugin --help: skills, agents, hooks, MCP, LSP), and I verified locally that 1.0.86 exposes no hook-disable flag — so with no hook material on disk, no hooks load. COPILOT_HOME passthrough is restored, keeping both login-session and token auth working; verified live on real 1.0.86 (audit passes on an empty plugin tree, inference succeeds) plus 4 new audit unit tests (tests/provider/test_copilot_cli.py::TestAuditCopilotHome: nonempty/empty/missing/default-path).

To keep this from being re-litigated per provider, I also added a "Provider CLI validation expectations" section to CONTRIBUTING.md (separate commit, revertable on its own) codifying the bar from this thread and #536: exact-version preflight before stdin on every completion path, no-hook-material enforcement (isolation where usable, presence-refusal otherwise), adversarial fake-host tests, synthetic-version gate tests, no silent fallbacks. If you know a hook path outside installed-plugins/, I'll extend the audit to cover it.

Yoseph-Zuskin and others added 2 commits September 19, 2026 10:41
…e homes

- Add _preflight_copilot_policy as the copilot CliSpec preflight:
  [binary, --version] under the isolated child env rejects anything
  but exactly 1.0.86 before run_agent_cli delivers stdin — covers
  normal scans and direct complete() calls, which never hit the
  once-per-scan availability probe
- _prepare_copilot_env redirects HOME/USERPROFILE/COPILOT_HOME to
  per-invocation temp dirs (user/plugin lifecycle hooks have no argv
  off-switch); auth survives only via forwarded token vars, persistent
  login sessions are no longer carried over
- Tests: 5 preflight mocks incl. synthetic 9.9.99, home-redirect unit
  test, fake-host OPERATOR_HOME bait test; stale COPILOT_HOME
  expectations rewritten
- Verified: file suite 46 passed / 3 skipped; provider-wide 87 passed /
  6 skipped; ruff check + format + diff-check clean

Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com>
Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai>
- Codifies the NVIDIA#536/NVIDIA#572 review bar as entry criteria: exact-version
  preflight before stdin on every completion path, no-hook-material
  enforcement (isolation where usable, presence-refusal otherwise),
  adversarial fake-host tests, synthetic-version gate tests, no
  silent fallbacks
- Kept as a standalone commit so it can be reverted on its own if
  maintainers prefer this guidance elsewhere

Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com>
Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Github Copilot LLM as a provider

3 participants