[Yoseph-Zuskin]

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, #680

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.105 fails. No status subcommand exists; authentication works
  via login session or COPILOT_GITHUB_TOKEN (_prepare_copilot_env
  re-reads only that token plus COPILOT_HOME, dropping every other
  COPILOT_*, all GITHUB_COPILOT_*, GH_TOKEN, and the enterprise
  token variables; the shared scrub strips them too).
  COPILOT_AUTO_UPDATE is deliberately neither forwarded nor forced:
  disabling updates selects an older cached runtime generation (probed
  2026-09-23), which would brick the version pin; mid-scan drift is
  covered by the per-completion version preflight instead.
• Per-completion preflight re-verifies copilot --version under the
  isolated child env on every call (a swapped/updated binary never
  receives scan content), then audits the copilot home
  (installed-plugins/, extensions/, hooks/*.json, inline hooks
  plus enabledPlugins/extraKnownMarketplaces in settings.json,
  user mcp-config.json/lsp-config.json, providers.json), custom
  agent files (agents/**/*.md, nested included — a truthy frontmatter
  hooks map or unparseable frontmatter refuses, covering 1.0.97's
  agent-scoped declarative hooks), and the fresh temp working dir
  (.github/agents joins the repo-level hook tripwires) — inference
  refuses when hook/server material is present. No model registry (CLI
  fallback, per the registry-split lesson).
• Home/XDG transport boundary: COPILOT_HOME is forwarded
  scanner-resolved absolute (no trim, no tilde expansion), so the
  audit and the child name the same tree for relative and
  trailing-space forms. Inherited XDG_CONFIG/STATE_HOME never
  reach the child (shared allowlist scrub strips them); the audited
  defaults apply. Transport regressions use an env-recording
  stand-in through the real temp-cwd path.
• Deliberate exception worth flagging: _prepare_copilot_env preserves
  only COPILOT_GITHUB_TOKEN through the scrub (everything else
  COPILOT_*, all GITHUB_COPILOT_*, GH_TOKEN, GH_ENTERPRISE_TOKEN,
  and GITHUB_ENTERPRISE_TOKEN are dropped). Rationale: it is the CLI's
  documented headless auth path — without it, token-only CI setups cannot
  use the provider at all, and the issue being resolved is precisely
  keyless operation. A CI gh workflow token must never become the
  agent's credential. The model itself gets no tools, so it cannot read
  the environment; the CLI redacts these variables from its own output by
  default. Residual (inherent to Copilot auth): the CLI falls back to
  gh auth token when no other credential is available, so a stored gh
  login can still reach the child. 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: 114 passed, 11 POSIX-skipped
  (TDD: argv, auth, parser, wiring, registry label, agent-file hook
  audit, adversarial). Combined with tests/unit/test_agent_cli.py:
  199 passed / 11 skipped on c577a0d7.
  Linux-CI follow-up fixed in-file: the fake copilot stub read
  ATTACK_MARKERS/FAKE_COPILOT_VERSION from the scrubbed child
  env and crashed with KeyError (POSIX-gated, never seen on
  Windows); markers path and version are now baked into the stub
  source at write time, mirroring test_opencode_cli.py. No
  product-code change.
• 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 (tests/provider, tests/unit/test_agent_cli):
  271 passed / 17 skipped / 4 deselected on this head.
• ruff check + ruff format --check: clean on all ten touched Python files
  (_agent_cli.py, _agent_cli_base.py, providers/__init__.py,
  copilot_cli/{__init__,provider}.py, cli.py, llm_utils.py,
  inference_usage.py, tests/provider/test_copilot_cli.py,
  tests/unit/test_agent_cli.py).
• Pin re-verified on the real 1.0.105 runtime (2026-10-28): full --help
  flag surface present (incl. -s, --model, no hook off-switch), the
  installed 1.0.105 binary parses and passes the version gate, and the
  six argv flags still exist. The exact pin stays deliberate: this bump
  admits 1.0.105 only, not a range; 1.0.104 is still refused because it
  was skipped, not re-verified. Provider
  argv does not pass --trust-folder (added 1.0.100) or any
  team/plugin activation surface (1.0.101's --plugin-dir and plugin
  subcommand are unused), so the fresh temp cwd stays untrusted and no
  user/team plugin is activated by argv; the existing preflight audit
  still refuses a non-empty installed-plugins/ tree. 1.0.100-1.0.105 add
  --trust-folder, --disable-client-telemetry, --sync-session, and
  hosting flags, and retire the legacy --remote/--no-remote flags
  (never used here); --trust-folder is deliberately not passed. No
  1.0.93-1.0.105 release note announces a change to the audited local
  hook source order or XDG migration sets; 1.0.100/1.0.101 add startup
  plugin activation, documented as an explicit residual under Risks. The
  live nonce round-trip (2026-10-28: answered exactly via the provider's
  run_agent_cli temp cwd and from a repo working dir, -s stdout
  carries only the response text) is proven on 1.0.105; a file-creation
  probe was refused with zero side effects under the same argv. The
  1.0.92 release-source note stays (static source analysis for the
  audited local sources, pinned deliberately).
• Merged upstream/main (d501c0d5, 2026-10-28), committed as
  c577a0d7; the merge left the tree clean and the merge commit unamended.
  Two doc-only edits in this branch are listed below. The only
  conflict (README.md, provider/CLI notes reworded by both sides) was resolved
  to the union — upstream's multi-version opencode_cli note kept verbatim
  alongside this branch's copilot_cli pin and gh-token residual.
• Two doc-only corrections in docs/DEVELOPMENT.md found while verifying the
  merge: the llm_analyzer_base source link is restored to
  src/skillspector/llm_analyzer_base.py (this branch had reverted upstream
  1d09e807 to a nonexistent nodes/ path), and the copilot_cli token
  sentence is folded back into the single CLI-providers blockquote. No
  behavior change.
• The _agent_cli.py docstrings now record the local audit provenance
  and the account/team startup boundary; this changes comments only.
• Diff vs upstream/main: 14 files changed, 2282 insertions(+),
  24 deletions(-) (1487 of the additions are
  tests/provider/test_copilot_cli.py).

Sample

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

Risks

• Account/team-managed plugin activation is residual. 1.0.100 installs
  and updates trusted-repository plugins before chat, but that path is
  gated on folder trust; the fresh mkdtemp cwd is untrusted,
  --trust-folder is not passed, and COPILOT_ALLOW_ALL is not set, so
  it is inert. 1.0.101 can assign Team plugins from Team-defined
  marketplaces; the CLI fetches that assignment from the account and may
  install missing plugins during its best-effort startup preparation,
  after the local preflight. The preflight can audit installed-plugins/
  before startup but cannot see an account assignment or a startup
  download. This is an explicit account/administrator-policy residual,
  not a claim that the startup tree is hook-free; PIC acceptance is
  requested.
• 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 behavior: silent acceptance); the version
  gate pins exactly 1.0.105 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]

[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.

[yashrajp22]

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.

[rng1995]

[SkillSpector Review]

Re-reviewed exact head c9e001f29c7108fd3ecc23f7b6c4f15cac6fa53c, including the complete diff, provider/transport call path, tests, previous reviews and author replies, and CI.

Previous findings:

• Addressed: custom-instruction discovery is explicitly disabled with --no-custom-instructions; built-in MCPs and auto-update are also disabled, with argv coverage.
• Addressed in implementation: the exact-version check is now a registered per-completion preflight, invoked before Popen/stdin delivery, including direct complete() calls. The unsupported-version test currently calls the helper directly; please also exercise the public completion path so a future registry/wiring regression cannot bypass the gate unnoticed.
• Still open: user/plugin hook isolation is only partially addressed. Rejecting nonempty installed-plugins/ does not reject user hook files or inline settings hooks. The inline finding supplies the concrete non-plugin paths requested in the author reply.

I inspected (did not execute) package/app.js and the version metadata from the official Copilot CLI 1.0.86 release, asset github-copilot-1.0.86-darwin-arm64.tgz. createNativeHookSession loads user settings and passes both settingsJson and userHooksDir to hookSessionCreate before adding plugin hooks separately. This is pinned-runtime evidence, not an assumption from newer documentation.

All six reported CI checks pass. Contributor code/tests and authenticated Copilot inference were not executed during this review; the remaining finding is based on source inspection. Approval remains blocked on closing and testing the non-plugin hook paths.

[rng1995]

[SkillSpector Review]

Re-reviewed current head 05f3e3236ea5eae91ffa611faf1df4b5ba239a52, both follow-up commits, the current provider/transport code and regression tests, prior discussions, and CI.

The previous direct-home cases are now addressed: the audit rejects user hooks/*.json, inline settings.json hooks, unreadable settings, and missing explicit homes; the temp-directory tripwire and transport-level rejection test are also present. The custom-instruction/MCP restrictions and per-completion exact-version preflight remain wired in. I also independently checked the official Copilot 1.0.88 launcher: I am not requesting changes solely because auto-update was re-enabled.

One remaining user-hook path prevents approval: the new audit observes the home before Copilot's inference startup migrates legacy XDG hook material into it. The --version subprocess exits before those migrations, so it cannot make the subsequent home audit sufficient. This is a deterministic startup-order issue, not a hypothetical concurrent modification. The inline comment gives the affected configuration, exact pinned-runtime source path, and required correction.

Evidence: read-only inspection of package/app.js from the official Copilot CLI 1.0.88 release, asset github-copilot-1.0.88-darwin-arm64.tgz. The version branch returns before tOn(); tOn() calls X0n for XDG config/state migrations; normal session creation subsequently loads the destination home's hooks.

All six reported CI checks pass. Contributor code, tests, and downloaded Copilot code were not executed locally; this finding is source-traced. No merges or thread-resolution changes were made.

[Yoseph-Zuskin]

Hi @yashrajp22 and @rng1995, all comments have been addressed. Please resolve them or let me know if you would prefer I do so myself. The PR description has also been updated to reflect that the pinned CLI version is now 1.0.88. I will propose a minor automation to check for OpenCode and GitHub Copilot CLI releases once a week after this and #613 will be merged.

[rng1995]

[SkillSpector Review]

Re-reviewed exact head a053ab575bdf23e46d5971af3aedeffc2f11a75b, including the complete fourteen-file diff, transport/provider integration, tests, all prior reviews/replies/thread states, and CI. I also inspected the official Copilot CLI 1.0.88 release source as inert text.

Addressed: explicit custom-instruction/built-in MCP restrictions remain in place; the exact-version preflight is registered before inference Popen; current/fallback home hooks and inline settings are audited; the previously reported XDG_CONFIG_HOME user-hook migration source is now audited and has transport-level rejection coverage.

Still requires changes: startup migration is not fully covered. The pinned runtime also migrates installed-plugins from XDG_STATE_HOME/.copilot, but this environment variable survives the child environment and its source tree is never audited. The new inline finding describes how plugin hook material can appear after the audit and how to close this path. This is a separate migration source from the CONFIG-hook example fixed in the previous thread, not a repetition of that resolved example.

The earlier requested public-path unsupported-version regression is also still absent: the 9.9.99 rejection test invokes _preflight_copilot_policy directly, while transport tests run the supported version. Please add that public completion rejection case while extending migration coverage, so removal of the registered preflight cannot silently bypass the gate.

All six reported hosted checks pass. Contributor code/tests, downloaded runtime code, and authenticated Copilot inference were not executed locally. No merge or thread-resolution changes were made.

[rng1995]

[SkillSpector Review]

Hi @Yoseph-Zuskin, thank you for the quick follow-up on the XDG state migration audit and the public-path version test!

Value and readiness: This PR adds a copilot_cli provider, so GitHub Copilot users can run semantic analysis without an OpenAI or Anthropic key (#8). Every earlier finding from this thread and from @yashrajp22's review is resolved on the current head. One new gap blocks approval: Copilot CLI personal extensions are another source of ambient code, and the preflight audit does not cover them. Once that is closed, the remaining questions (exact pin versus Copilot's release cadence, and token forwarding) are PIC decisions, not code defects.

Previous findings:

• Custom-instruction discovery (rng1995, 2026-09-18): Resolved. --no-custom-instructions is in _build_copilot_argv (_agent_cli.py:1009). The argv shape test and the fake host both assert it.
• Inference bypasses the exact-version check (@yashrajp22): Resolved. _preflight_copilot_policy is the registered CliSpec preflight (_agent_cli.py:1371-1378), and run_agent_cli calls it before Popen on every completion.
• Ambient user/plugin lifecycle hooks (@yashrajp22), plus user hook files and inline settings hooks (rng1995, 2026-09-22): Resolved for the hook sources GitHub documents. _audit_hook_tree refuses a non-empty installed-plugins/, hooks/*.json, an inline hooks block in settings.json, and an unreadable settings.json.
• Public-path unsupported-version regression (rng1995, 2026-09-22 and 2026-09-28): Resolved. test_public_completion_rejects_unsupported_version sends 9.9.99 through run_agent_cli. If the registered preflight were removed, the run would return normally and pytest.raises would fail.
• XDG_CONFIG_HOME hook migration (rng1995, 2026-09-23): Resolved.
• XDG_STATE_HOME installed-plugins migration (rng1995, 2026-09-28): Resolved. _audit_copilot_home now audits both the explicit $XDG_STATE_HOME/.copilot and the default ~/.local/state/.copilot (_agent_cli.py:1189-1198). Unit cases and test_xdg_state_source_rejects_before_prompt_delivery cover it: a clean home plus populated STATE plugins is rejected, and the marker directory stays empty.
• Upstream merge 1287da8: the PR's own change is identical before and after the merge, apart from context lines from main's OpenCode 1.18.32 bump.

Material findings

1. [Blocker] src/skillspector/providers/_agent_cli.py:1201: _audit_hook_tree does not check personal extensions.

   • GitHub's Copilot CLI config-directory reference lists ~/.copilot/extensions/ as "Personal extensions loaded by the CLI".
   • The official changelog for 1.0.41 says extensions load in prompt mode and user extensions load by default. Piped-stdin runs like this one are prompt-mode runs; 1.0.78 aligned their hook behavior with -p.
   • The CLI runs each extension as a forked Node.js process, and extensions can register tools and hook callbacks (changelog 1.0.3 and 1.0.85). They need experimental mode, which users can persist as "experimental": true in ~/.copilot/settings.json.
   • Failing configuration: a home with no plugins and no hooks, settings.json = {"experimental": true}, and ~/.copilot/extensions/x/extension.mjs. The audit passes, and every completion forks that code with scanner privileges and the inherited environment, including the forwarded GitHub token variables. The --available-tools allowlist does not control this. It is the same class of path as the hooks fixed in earlier rounds, and it contradicts the _audit_copilot_home docstring, which says every user and plugin source is checked.
   • Expected fix:
     • Refuse a non-empty extensions/ in every audited tree (home, default home, both XDG sources).
     • Add .github/extensions to _REPO_HOOK_PATHS.
     • Drop GITHUB_COPILOT_* in _prepare_copilot_env. Line 930 drops only COPILOT_*, so GITHUB_COPILOT_PROMPT_MODE_EXTENSIONS, GITHUB_COPILOT_PROMPT_MODE_REPO_HOOKS and GITHUB_COPILOT_PROMPT_MODE_WORKSPACE_MCP currently reach the child.
     • Add unit cases, plus a transport regression with a clean hook tree and a populated extensions/ that is rejected before prompt delivery.
   • Alternative: if the pinned 1.0.89 source shows extensions cannot load under this argv, cite that evidence instead (for example a verified gate you can force off).

2. [Non-blocking] User MCP and LSP servers: --disable-builtin-mcps disables only the built-in GitHub server. The same reference lists ~/.copilot/mcp-config.json and lsp-config.json, which define user server commands. Please confirm on 1.0.89 whether user MCP servers start on each completion under this argv. Their tools are hidden by --available-tools, but a local server is still a spawned command, and its env block can reference the forwarded token variables. Either audit them like hooks or document them as residual risk next to the policy-hook note. Listing the surfaces you checked in the docstring would make the next pin bump easier to re-verify.

3. [Non-blocking] Docs and comments disagree with the code:

   • src/skillspector/providers/copilot_cli/provider.py:27 says the argv includes --no-auto-update, which was deliberately removed, and it omits --disallow-temp-dir.
   • _build_copilot_argv has two "Deliberately NOT included" sections (lines 974 and 997).
   • The comment at tests/provider/test_copilot_cli.py:292 says a missing COPILOT_HOME is refused; _audit_copilot_home documents and implements the opposite.

4. [Non-blocking] src/skillspector/providers/_agent_cli.py:1067 and :1110: "1.0.89" is hard-coded in both gates and both error messages. Copilot ships a stable release about every two days. A single _COPILOT_SUPPORTED_VERSION constant, as opencode_cli uses, would stop a bump from updating one gate but not the other.

5. [Non-blocking] The tests are not hermetic. _audit_copilot_home always audits the real ~/.copilot, ~/.config/.copilot and ~/.local/state/.copilot, and several tests do not redirect HOME:

   • TestPreflightCopilotPolicy.test_pinned_version_passes
   • TestAuditCopilotHome.test_empty_plugins_pass
   • TestAdversarialTransport.test_adversarial_child_gets_no_usable_tools

   test_pinned_version_passes also audits the shared /tmp as its working directory. These tests fail on a contributor machine that has Copilot plugins or hooks installed. Add an autouse fixture that points HOME/USERPROFILE at tmp_path and clears COPILOT_HOME, XDG_CONFIG_HOME and XDG_STATE_HOME, and pass tmp_path as the working directory.

6. [Non-blocking] Two small fixes:

   • src/skillspector/cli.py:563-564: one extra leading space misaligns the scan --help text.
   • src/skillspector/providers/copilot_cli/__init__.py: the license header is missing the http://www.apache.org/licenses/LICENSE-2.0 line that every other file has.

PIC tradeoffs:

• Exact pin versus release cadence. Copilot CLI shipped seven stable releases between 2026-09-16 and 2026-10-01. 1.0.90 (2026-09-30) and 1.0.91 (2026-10-01) have already superseded the 1.0.89 pin. Auto-update is deliberately left on, so default installs leave the pin within days. copilot_cli then reports itself unavailable until SkillSpector ships a new pin. That failure is closed and visible, but those users get no semantic scans. The PIC should decide whether that level of availability is acceptable for a supported provider. If it is, re-verify and bump to the current stable release just before merge.
• Token forwarding. _scrub_env strips GITHUB_TOKEN for every other CLI provider. This provider re-injects GITHUB_TOKEN and GH_TOKEN along with COPILOT_GITHUB_TOKEN, and in CI GITHUB_TOKEN is often a repo-scoped token unrelated to Copilot. Forwarding only COPILOT_GITHUB_TOKEN (the one-line fallback the description offers) keeps headless Copilot auth without giving a general repo token to an agent process that reads untrusted content. GH_TOKEN already reaches all CLI providers because _scrub_env does not strip it; that gap predates this PR.
• Merge order. This PR and #665 change adjacent README lines. Whichever lands second needs a small rebase.

Verification and gaps: What I checked, statically:

• The full 14-file diff at the head, the two author commits since a053ab57 (98adf47, 87d0b4e), and the merge 1287da8. I compared the PR's change before and after the merge; only context lines from main differ.
• The call path run_agent_cli → _prepare_copilot_env → _preflight_copilot_policy → Popen, and the final argv.
• As text: GitHub's Copilot CLI hooks reference, config-directory reference and extension docs, and the official github/copilot-cli changelog through 1.0.91.

Gaps:

• I did not download or inspect the 1.0.89 runtime. Findings 1 and 2 rest on vendor documentation and the changelog, not on tracing the pinned source.
• Per policy, I did not run the tests or authenticated Copilot inference.

All six CI checks pass on this head.

---

Decision: Changes Requested (reviewed head 87d0b4e1bb3b9d79d6c2aa27e13b356c826b31f0)

[Yoseph-Zuskin]

Hi @rng1995, thank you for the thorough review. Three things in this push. First, the GH_TOKEN blocker is fixed at both layers (details in the inline reply), with the token test on the real _scrub_env() path. Second, the two unaudited surfaces are now audited: truthy enabledPlugins/extraKnownMarketplaces in settings.json refuse, and a non-empty providers.json refuses, all in the same six trees. The leftover {"mcpServers": {}} shape passes, since empty collections define no commands. Third, the small stuff: hermetic tmp_path preflight tests, duplicate heading removed, help and docstrings aligned to the single-token reality.

Also in this push: pin bumped to 1.0.92 (stable since 2026-10-05). Full --help surface rechecked against the installed runtime, no hook off-switch, and the real binary passes the gate. upstream/main (b6f0304) merged; the 4 conflicts were all provider-list unions with the new gemini provider. Green locally: copilot file 100/8, provider+unit 236/11, ruff clean.

Two PIC calls left, both one-liners in the docs now: blocking the whole provider on any user hook/plugin/server, and the gh auth fallback. @yashrajp22's 2026-09-19 request still needs a re-review or a maintainer dismissal. Guidance welcome on any of it.

[rng1995]

Re-reviewed all fourteen changed files, eleven historical inline threads, substantive prior review rounds, and the provider/transport trust boundary. The earlier feedback is implemented and the 246-case provider/transport suite passes (1 skipped, 4 deselected).

One new P1 remains: preflight audits a normalized XDG path relative to the scanner directory, while the pinned CLI resolves the unchanged value relative to inference's fresh temporary directory. Relative values and values ending in a space can therefore select an uninspected hooks/plugins migration tree. The inline finding contains reproducible transport evidence and source confirmation from the exact official 1.0.92 release. No authenticated inference or hook execution was performed. Please address this boundary before approval, and update the branch with main to resolve its current merge conflict.

[Yoseph-Zuskin]

Hi @rng1995, thank you for the thorough re-review and the transport evidence!

Latest push closes the P1: COPILOT_HOME is forwarded scanner-resolved absolute so the audit and the child name the same tree, and the CONFIG/STATE half is handled by adopting the merged upstream allowlist scrub (inherited XDG never reaches the child; the audited defaults apply), pinned by scrub and transport regressions. Details in the inline reply, including the refuse-vs-forward call.

Also in this push: pin bumped to 1.0.95 with the full --help surface rechecked on the installed runtime and the real binary passing the gate (two live-provenance lines stay cited at 1.0.92 until the live probes re-run), and upstream/main (5edc347c) merged — 6 conflicts, all provider-list unions around the codex_cli disable, resolved to the union. Green locally: copilot file 104/11, provider+unit 227/14, ruff clean.

Open items unchanged: the fail-closed availability posture and gh-fallback PIC calls, and @yashrajp22's 2026-09-19 request still needs a re-review or a maintainer dismissal. Guidance welcome on any of it.

[rng1995]

[SkillSpector Review]

@Yoseph-Zuskin thank you for closing the home-resolution boundary so precisely, with transport regressions that mirror the reported repro.

Value and readiness

The last blocker is resolved:

• COPILOT_HOME is forwarded scanner-resolved, absolute and untrimmed, so the audit and the child name the same tree.
• Inherited XDG_CONFIG_HOME / XDG_STATE_HOME can no longer reach the child, so the CLI uses the audited defaults.

The branch merges cleanly into current main (b07f181d), and all six checks pass on 783e4442. I have no further required code changes. It is ready for final maintainer/PIC review, subject to the version-pin decision below and @yashrajp22's outstanding change request.

Material findings

Prior findings:

• [P1] Audit the same XDG/home tree that inference resolves: resolved.
  • _prepare_copilot_env forwards os.path.abspath(COPILOT_HOME) with no strip and no expanduser (_agent_cli.py:935-943), and _copilot_home reads the identical value (:1143-1145).
  • XDG_CONFIG_HOME / XDG_STATE_HOME are not in the _RUNTIME_ENV_NAMES allowlist, so the CLI falls back to ~/.config/.copilot and ~/.local/state/.copilot. Both are audited unconditionally (:1221-1241).
  • test_relative_copilot_home_forwarded_absolute and the two …never_reaches_child cases exercise the real run_agent_cli temp-cwd path.
• Merge conflict with main: resolved. The branch is clean against b07f181d, including #806.
• Earlier rounds: still resolved. GH_TOKEN and the enterprise tokens are dropped (:861-865, :925-931), and the plugin-marketplace, BYOK, MCP/LSP and extensions audits are unchanged.

New (optional):

1. Optional, _agent_cli.py:1146,1216: the default-home branch uses os.path.expanduser("~"). A relative HOME or USERPROFILE therefore resolves against the scanner cwd for the audit, but against the fresh temp cwd in the child. This is the same class as the P1, but I don't consider it blocking: unlike XDG overrides, no legitimate setup uses a relative HOME, and an attacker who controls the scanner's environment already controls PATH. A cheap fix is to fail closed when the resolved home base is not absolute.
2. Optional, :1227-1231: this comment still says deleting the STATE variable "falls back to an unaudited default". The scrub now deletes it, and :1238-1241 audits that default, so please update the comment.

PIC tradeoffs

• The pin is already stale. Copilot CLI 1.0.96 and 1.0.97 are stable releases (1.0.97 on 2026-10-13), and auto-update stays on, so default installs will report copilot_cli unavailable at merge. Bumping is more than a string change: 1.0.97 adds "agent-scoped declarative hooks for custom agents", a new hook source the audit inventory would need to cover first. That inventory is source-verified at 1.0.92; the 1.0.93–1.0.95 release notes show nothing new, and 1.0.93 narrows user settings to settings.json.
• Fail-closed availability (unchanged): any user hook, plugin, extension, MCP/LSP server or BYOK entry disables the provider.
• gh auth token fallback (unchanged): this residual is documented in README and DEVELOPMENT.md.

Verification and gaps

• This round's delta: I read 606c2b57..783e4442 in full (_agent_cli.py, tests, README, DEVELOPMENT.md) and the upstream merge.
• Call path traced: run_agent_cli → _scrub_env → _prepare_copilot_env → _preflight_copilot_policy → _audit_copilot_home, for COPILOT_HOME, XDG and HOME.
• Vendor notes: I read the Copilot CLI release notes for 1.0.93–1.0.97 as text.
• Unchanged code: I relied on the earlier full 14-file rounds and spot-checked it here.
• Not done: I did not run tests or any Copilot runtime.
• Merge order: #807 edits the adjacent docs/DEVELOPMENT.md line, so whichever of the two merges second needs a trivial conflict fix.
• Other reviewer: @yashrajp22's 2026-09-19 CHANGES_REQUESTED still needs their re-review or a maintainer dismissal.

[Yoseph-Zuskin]

A status recap to make re-review easy, plus one question below.

Since the September change request, every finding from the earlier rounds
is addressed on the branch: COPILOT_HOME is forwarded scanner-resolved
absolute so the audit and the child name the same tree, inherited
XDG_CONFIG_HOME/XDG_STATE_HOME cannot reach the child, the home audit
covers all six trees plus the XDG migration sources, GH_TOKEN and the
enterprise token variables are dropped, and a per-completion preflight
re-verifies the runtime version before any prompt bytes move. rng1995
confirmed the last blocker cleared. Current main (including #849) is
merged and the one conflict (docs/DEVELOPMENT.md, provider list) is
resolved.

@yashrajp22, would you be able to take another look when you have a
moment? One optional item remains from review (relative HOME /
USERPROFILE expansion in the default-home fallback); say the word and
I will add the fail-closed guard in this PR, otherwise I will track it as
a follow-up.

The new commit bumps the pin to the current stable release and covers
1.0.97's agent-scoped declarative hooks: custom agent files
(agents/*.md, user and repo level, nested included) can now declare a
hooks map in YAML frontmatter, so the audit refuses a truthy hooks
map or unparseable frontmatter in agent files, and .github/agents joins
the temp-cwd tripwires.

Question for the maintainers: the exact-version pin is already stale in
practice, because auto-update moves most installs past the pinned build
while the gate keeps failing closed. Would you prefer the next commit to
declare a supported version range (same fail-closed behavior outside it),
or should that be deferred to a follow-up issue and PR? Happy with
either.

Local verification on this head: copilot suite 114 passed, ruff clean,
and the --help flag surface re-verified on the new runtime. The live
nonce re-probe on 1.0.98 follows before final merge.

[Yoseph-Zuskin]

Hi @rng1995 and @yashrajp22,

Thanks @rng1995 for approving the pre-bump head. Flagging that last push
only moves the pin, from _COPILOT_SUPPORTED_VERSION 1.0.98 to 1.0.103
(npm latest, 2026-10-22; 1.0.99 never shipped), and re-verifies the
surface we depend on. The one new security-relevant flag in that window,
--trust-folder (1.0.100), is deliberately not passed, so the fresh temp
cwd stays untrusted. All argv flags this provider relies on are still
present; 1.0.103 retires the legacy --remote/--no-remote flags, which
we never used. Tests: 114 passed / 11 POSIX-skipped, ruff clean. The live
nonce round-trip now passes on 1.0.103 from both a repo and a temp working
dir (-s stdout carries only the response text), so the two live
citations now cite 1.0.103.

One scope question: would you be open to replacing the exact pin with an
explicit verified-version frozenset in this PR covering every version
this branch has pinned since its first commit
({"1.0.85", "1.0.86", "1.0.88", "1.0.89", "1.0.91", "1.0.92", "1.0.95", "1.0.98", "1.0.103"}),
with the per-version evidence already accumulated on the branch, modeled
on the opencode_cli follow-up (#712/#713)? That keeps this an explicit set
rather than a range, so no unverified release is admitted. Or would you
prefer to keep this PR as the minimal bump and defer that to a follow-up
issue and PR? I am happy with either; my default is to defer so this review
stays small, but I can fold it in here if you would rather review it once.

[rng1995]

[SkillSpector Review]

Value and readiness

Manual Review Needed at 1a8bde14e48845946e602b7c0edc6daf43bce558. The earlier custom-instruction, per-completion version, hook/migration, extension, token and home-resolution findings are implemented, and all 13 historical threads are resolved. The new agent-frontmatter audit also addresses the later agent-scoped hook surface. I cannot carry the earlier approval across the runtime expansion without settling the startup sources described below.

Material findings

The remaining uncertainty is specific to the pinned runtime, rather than the absence of local test execution. src/skillspector/providers/_agent_cli.py:1189 describes its hook inventory as verified against 1.0.92, while the actual gate now admits 1.0.103. The description says 1.0.99–1.0.103 introduce no sourcing changes, but the official 1.0.100 notes describe automatic trusted-repository plugin installation before chat, and 1.0.101 adds Team-assigned plugins from Team-defined GitHub marketplaces. These are relevant to a presence audit that runs before inference startup. Establish from the exact runtime source whether managed/team/repository activation can introduce hook-capable material after the clean-home checks, and provide focused evidence for the pinned piped-stdin/fresh-directory invocation. If those paths are intentionally covered by the administrator-policy residual, state that boundary explicitly for PIC acceptance. This is an unresolved assurance gap, not a reproduced hook-execution finding.

The earlier required fixes remain resolved: argv disables custom instructions/built-in MCPs; the registered preflight checks every completion before prompt delivery; home/default/XDG trees cover user hooks/plugins/extensions/server/BYOK material; unrelated GH credentials are dropped; the explicit home is absolute and untrimmed; inherited XDG variables are absent. The relative-HOME edge case from the last review remains a previously optional limitation.

PIC tradeoffs

The latest stable upstream release is already 1.0.105 (2026-10-28), following 1.0.104 on October 27. The exact 1.0.103 gate deliberately refuses those versions, so normal auto-updated installs may report this provider unavailable. Decide whether to ship that verified pin or verify a newer one. The stored gh-auth fallback and administrator-owned policy hooks remain documented residuals. Another reviewer's historical CHANGES_REQUESTED still requires their re-review or an authorized maintainer disposition.

Verification and gaps

Read the full current PR diff and pinned transport/provider/test source; traced prepare_env → registered preflight → stdin delivery; reviewed all historical inline findings and author replies plus the latest full approval. Static tests cover hostile environment/argv, version rejection before stdin, hook/migration/server/extension refusal and home-resolution transport. All six hosted checks succeeded. Independently read the official 1.0.100, 1.0.101, and subsequent release notes with gh. I did not inspect the exact 1.0.103 packaged runtime implementation or execute any contributor/vendor runtime or tests; consequently the newly documented activation paths are not established safe by this review.

[Yoseph-Zuskin]

Hi @rng1995 and @yashrajp22,

Your review is against 1a8bde14, which is why the 1.0.103 tradeoff is in
it. The branch now includes c577a0d7, which pins 1.0.105 after live
verification and merges upstream/main (d501c0d5), plus a doc-only commit
with the boundary text. The pin stays exact: 1.0.104 was skipped and is
still refused.

On the 1.0.100/1.0.101 activation point, I inspected the actual 1.0.105
packaged runtime (...\copilot\pkg\win32-x64\1.0.105\cli-main.js), not just
the release notes. The two startup paths split:

• Trusted-repository install/update (1.0.100) is gated on folder trust
  (folderTrustIsTrusted). This provider runs in a fresh mkdtemp cwd,
  never passes --trust-folder, and does not set COPILOT_ALLOW_ALL, so the
  cwd is untrusted and that path is inert.
• Team-assigned plugins (1.0.101) are fetched from the account
  (authTeamPluginAssignmentsForSession) and passed as teamMarketplaces
  into best-effort plugin preparation. Missing plugins install into the
  audited installed-plugins/ tree after startup, so the preflight cannot
  see an account assignment or a startup download. The preflight still
  refuses a non-empty installed-plugins/ tree before startup, but it cannot
  prove the startup tree will stay clean.

I am not claiming the startup tree is hook-free. The new doc-only commit
records this as an explicit account/administrator-policy residual in
_audit_copilot_home, and I updated the PR description to replace the
overbroad "no sourcing changes" wording and list the residual under Risks.
This is documentation, not speculative local checks. Could the PICs accept
that boundary as the intended assurance level for this PR? If not, I can
pursue a deeper source-level proof for Team activation next.

On provenance: the local hook source order and XDG sets were audited at
1.0.92; no 1.0.93-1.0.105 release note announces a change, and the 1.0.105
packaged runtime still uses the same local plugin/hook tree names. The
docstrings now state that instead of implying a fresh source audit of 1.0.105.

I also fixed two docs/DEVELOPMENT.md issues found during the upstream
merge: the llm_analyzer_base link pointed at a nonexistent nodes/ path,
and a stray line break split the CLI-providers note. Both are doc-only and
unrelated to your finding.

c577a0d7 also keeps the README conflict resolution as a union, so
upstream's multi-version opencode_cli note and this branch's copilot_cli
note are both present.

No hook execution was observed. Tests: test_copilot_cli.py 114 passed / 11
POSIX-skipped; tests/provider + tests/unit/test_agent_cli.py 271 passed /
17 skipped / 4 deselected; ruff check and format clean.

The earlier frozenset scope question is still a possible follow-up; my
default remains keeping this PR a minimal exact pin.

[rng1995]

[SkillSpector Review]

Value and readiness

Manual Review Needed at 2ac8682ca0943b92e01c155140df74b75f258df8. The author has clarified the startup boundary and updated both runtime gates/tests to the currently published 1.0.105 pin. The earlier stale-pin concern is resolved, and the existing local hook, extension, migration, token and home-resolution fixes remain intact.

Material findings

The prior assurance question is now answered explicitly, but the acceptance decision remains open. src/skillspector/providers/_agent_cli.py:1308-1320 and the PR Risks section acknowledge that Team-assigned plugins can be fetched and installed during startup after the local preflight. A clean local installed-plugins/ tree therefore does not establish the provider's no-hook-execution boundary for those accounts. The code now correctly limits its absence guarantee to local material; it does not prevent that acknowledged account-controlled activation path. The author explicitly requests PIC acceptance of this residual. Record that decision, or provide an enforcement mechanism/proof that excludes such startup activation, before treating the provider as merge-ready. This is the same unresolved boundary from the previous review, rather than a new reproduced execution finding.

All historical local-source blockers remain resolved: custom instructions and built-in MCPs are disabled; the registered version preflight runs before stdin on every completion; default/home/XDG trees are audited; unrelated GH credentials and inherited XDG variables are removed; explicit Copilot homes are absolute and untrimmed; agent-frontmatter hooks are checked. All 12 conversations returned in the current thread snapshot are resolved. The previously optional relative-HOME limitation remains optional.

PIC tradeoffs

The current exact pin admits 1.0.105 only and still fails closed on unverified versions. Local audit provenance remains the 1.0.92 source inventory, supplemented by the author's later runtime/help/live checks. Account/administrator-controlled startup plugins, machine policy hooks and the documented stored gh-auth fallback remain separate residuals for acceptance. Another reviewer's historical CHANGES_REQUESTED still needs their re-review or authorized maintainer disposition.

Verification and gaps

Compared all changed source/tests against the prior completed assessment, inspected the complete current PR diff and upstream integration, reviewed earlier findings/replies, and rechecked the explicit local-versus-account startup documentation. Official gh release metadata confirms 1.0.105 as the current stable release. All six hosted checks succeeded on this head. The author's nonce and file-creation probes are reported evidence; I did not reproduce them or inspect/execute the packaged vendor runtime. No contributor code or tests were executed.