chat: support selective customization lockdown - #332071
chat: support selective customization lockdown#332071Harald Kirschner (digitarald) wants to merge 3 commits into
Conversation
|
Dependency status: microsoft/vscode-policy-watcher#75 now has green GitHub and Azure compile/package jobs. Current CI failures are the expected npm 404 for unpublished @vscode/policy-watcher@1.5.0; I will regenerate the lockfile and rerun immediately after that package is published. |
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
There was a problem hiding this comment.
Pull request overview
Adds selective enterprise lockdown for standalone chat customizations while preserving plugin-provided customizations and boolean behavior.
Changes:
- Supports validated selectors for skills, agents, hooks, and MCP.
- Enforces MCP policy during discovery, resolution, and active connections.
- Adds scalar-union managed-setting transport and coverage.
Show a summary per file
| File | Description |
|---|---|
src/vs/workbench/services/policies/test/browser/accountPolicyService.test.ts |
Updates managed-setting test types. |
src/vs/workbench/services/accounts/test/browser/managedSettings.test.ts |
Tests selective response adaptation. |
src/vs/workbench/services/accounts/browser/managedSettings.ts |
Types selective server responses. |
src/vs/workbench/contrib/mcp/test/common/mcpRegistry.test.ts |
Tests selective MCP enforcement. |
src/vs/workbench/contrib/mcp/common/mcpServer.ts |
Blocks unavailable policy-filtered collections. |
src/vs/workbench/contrib/mcp/common/mcpRegistry.ts |
Applies MCP-specific lockdown checks. |
src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts |
Tests selective agent and hook filtering. |
src/vs/workbench/contrib/chat/test/common/customizationLockdown.test.ts |
Tests selector and fail-closed semantics. |
src/vs/workbench/contrib/chat/common/plugins/AGENTS_PLUGINS.md |
Documents selective lockdown behavior. |
src/vs/workbench/contrib/chat/common/customizationLockdown.ts |
Implements per-surface policy evaluation. |
src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts |
Registers union-typed managed policy transport. |
src/vs/platform/policy/test/node/nativeManagedSettingsService.test.ts |
Tests native union definitions and watcher replacement. |
src/vs/platform/policy/test/common/fileManagedSettingsService.test.ts |
Tests selective normalization. |
src/vs/platform/policy/test/common/copilotManagedSettings.test.ts |
Tests union projection and parsing. |
src/vs/platform/policy/node/nativeManagedSettingsService.ts |
Passes unions to the native watcher. |
src/vs/platform/policy/common/copilotManagedSettings.ts |
Defines selectors, normalization, and fail-closed parsing. |
src/vs/platform/configuration/test/common/policyConfiguration.test.ts |
Tests structured internal boolean-policy values. |
src/vs/base/common/policy.ts |
Adds managed-setting scalar unions. |
package.json |
Requires policy-watcher 1.5.0. |
package-lock.json |
Updates the locked native dependency. |
.github/skills/policy-and-managed-settings/github-managed-settings.md |
Documents hybrid scalar/structured settings. |
Review details
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 20/21 changed files
- Comments generated: 3
- Review effort level: Balanced
|
Addressed all three Copilot review comments in 8267643. Focused native-policy and MCP suites pass (63 tests), and |
|
Temporarily pinned the dependency to the reviewed policy-watcher #75 commit in 22d787d (will replace with the published 1.5.0 package before ready review). This removes the npm 404 so draft CI can validate the full VS Code change now. |
|
Correction: the temporary dependency pin is commit 957df15. |
|
Removed the temporary tarball pin because Azure's dependency-cleanliness gate rejects it. Draft CI now uses the current published 1.4.0 package so all source/tests can validate; the final 1.5.0 manifest+lock bump remains gated on policy-watcher #75 merge/publication. |
|
The macOS policy export timeout exposed watcher 1.4 rejecting union definitions. Added a narrowly detected compatibility fallback in d09d026: 1.4 continues watching the first type (�oolean), while 1.5 receives the full union. Added regression coverage; 14 native-policy tests and ypecheck-client pass. Windows/macOS smoke failures were unrelated workbench-startup flakes and will be rerun after this push. |
Mohammad javad Dianat (dianatofficial)
left a comment
Mohammad javad Dianat (dianatofficial)
left a comment
There was a problem hiding this comment.
Solid PR. Tests cover the important paths.
|
Dependency unblocked: policy-watcher #75 merged and 1.5.0 is published. Merged current main, resolved managed-settings conflicts, updated package/lock metadata to the real npm artifact, reran policy export (clean), and passed 271 targeted tests. Fresh CI is running; I will mark ready after it is green. |
6e94e7f to
9f108e4
Compare
|
Refreshed onto latest main via rebase (head 9f108e4). Resolved managed-settings conflicts by preserving upstream freshness/disable helpers and current dependency versions while retaining selective lockdown logic. Dropped obsolete temporary dependency commits; the branch is now three focused commits atop main. Validation: 275 targeted tests pass, |
Preserve boolean-or-selective strictPluginOnlyCustomization values through managed settings and enforce individual skill, agent, hook, and MCP surfaces without weakening plugin provenance.\n\nAdd native scalar-union policy delivery, fail-closed validation, dynamic MCP revocation, focused tests, and enterprise documentation.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tighten scalar-union typing, preserve active native policy delivery across failed watcher replacement, and reject same-ID MCP collection swaps during async resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Retry scalar-union declarations with their first scalar type only when the installed native module reports the legacy type contract. This preserves existing boolean policy delivery until watcher 1.5.0 is installed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
9f108e4 to
676d01c
Compare
|
Main advanced by one commit while CI was finishing. Rebasing again completed without conflicts; PR head is now 676d01c directly atop the latest base. No feature changes were required. Fresh checks are running. |
|
Refresh complete. The branch is exactly three focused commits atop current main (�bb16775f3ac), with no unresolved review threads. Rebase validation: 275 targeted tests pass and policy export is clean. Every concrete required check passes, including Compile & Hygiene, Dependencies, screenshots, Monaco, Linux/macOS/Windows browser/Electron/smoke/remote, and Copilot tests. The only remaining blocker is requested code-owner review from joshspicer; the aggregate VS Code PR Check remains pending with |
|
I found a possible regression for non-enterprise users too in The new check treats a missing MCP collection as an enterprise customization policy block. However, For users, that can interrupt an active tool call and incorrectly report “Blocked by enterprise customization policy.” I reproduced the base/head difference using the policy-evaluation and disposal callbacks with stubbed observable notifications, rather than a full end-to-end run. Automatic reconnection afterward was not established. Could we distinguish an explicit policy denial from a temporarily absent collection, leave ordinary removal/replacement to lifecycle reconciliation, and add a live-connection regression test for a same-ID collection refresh with lockdown disabled? |
TL;DR: This PR lets admins require plugins for specific customization types, rather than locking down everything at once.
strictPluginOnlyCustomization: trueblocks standalone skills, agents, hooks, instructions, and MCP servers.["skills", "mcp"], to block only standalone skills and MCP servers. Plugin-provided customizations remain eligible, subject to other policies.Boolean
truekeeps its existing behavior. Selective arrays do not block instructions, and malformed arrays are intended to fail closed.Fixes microsoft/vscode-internalbacklog#8826
Dependency microsoft/vscode-policy-watcher#75 is merged and
@vscode/policy-watcher@1.5.0is published.Summary
Adds the canonical selective form of
strictPluginOnlyCustomizationwhile preserving existing boolean behavior:truecontinues to block standalone skills, agents, hooks, instructions, and MCP servers.skills,agents,hooks, and/ormcpblock only those standalone surfaces.trueremains the instruction lockdown.Implementation
boolean) when the legacy addon rejects a unionReview fixes
Addressed all automated review findings:
Validation
mainand resolved managed-settings changes from upstreamnpm run export-policy-data(clean generated policy result; named policy remains boolean)Local
typecheck-clientafter merging main is blocked by unavailable private SDK packages in this checkout; the pre-merge typecheck passed, and the fresh PR Compile & Hygiene job is the authoritative post-merge validation.