Add policies for more terminal chat settings #285732
Conversation
- Added policy for chat.tools.terminal.autoApprove - Added policy for chat.tools.terminal.ignoreDefaultAutoApproveRules - Updated policyData.jsonc with new policies - Updated test fixtures (in progress - need to fix fr-fr ADML formatting) Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com>
- Updated Windows ADMX and ADML files (en-us and fr-fr) - Updated macOS mobileconfig and plist files (partially) - Updated Linux policy JSON - Updated policyConversion.test.ts with new policies - 3 test failures remaining (macOS plist formatting) Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com>
- Fixed French ADML string order - Fixed plist extra newlines - Updated ChatAgentMode descriptions - 126 tests passing, 2 macOS plist fixture format issues remain Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com>
Tyriar
requested review from
joaomoreno,
joshspicer,
pwang347,
rebornix and
sandy081
as code owners
build/lib/test/fixtures/policies/darwin/com.visualstudio.code.oss.mobileconfig
Show resolved
Hide resolved
| "ChatMCP": "none", | ||
| "ChatPromptFiles": false, | ||
| "ChatToolsAutoApprove": false, | ||
| "ChatToolsEligibleForAutoApproval": "", |
There was a problem hiding this comment.
@pwang347 should this be the default JSON value that was generated when I ran ./scripts/code-cli.bat -- export-policy-data?
There was a problem hiding this comment.
yes, I think this is due to #277319 which was existing behaviour but something we should improve
There was a problem hiding this comment.
Pull request overview
This PR adds policy support for two terminal chat agent tools settings: chat.tools.terminal.autoApprove and chat.tools.terminal.ignoreDefaultAutoApproveRules. The ignoreDefaultAutoApproveRules setting is also being promoted from experimental to stable by removing its experimental tag.
Key Changes
- Added policy configuration for
chat.tools.terminal.autoApprovewith comprehensive default rules - Added policy configuration for
chat.tools.terminal.ignoreDefaultAutoApproveRules - Removed the
experimentaltag fromignoreDefaultAutoApproveRulesto promote it to stable - Updated policy description for
chat.agent.enabledacross localization files - Added test fixtures and policy definitions across all platforms (Windows, Linux, macOS)
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts | Added policy definitions for autoApprove and ignoreDefaultAutoApproveRules settings; removed experimental tag from ignoreDefaultAutoApproveRules |
| build/lib/test/policyConversion.test.ts | Added test cases for the new terminal policies and updated ChatAgentMode description |
| build/lib/test/fixtures/policies/win32/fr-fr/CodeOSS.adml | Added French localization entries for new policies (some strings remain in English) |
| build/lib/test/fixtures/policies/win32/en-us/CodeOSS.adml | Added English localization entries and version definitions for new policies |
| build/lib/test/fixtures/policies/win32/CodeOSS.admx | Added ADMX policy definitions for Windows Group Policy |
| build/lib/test/fixtures/policies/linux/policy.json | Added Linux policy entries with default values |
| build/lib/test/fixtures/policies/darwin/fr-fr/com.visualstudio.code.oss.plist | Added macOS policy entries for French locale (with XML structure errors and untranslated strings) |
| build/lib/test/fixtures/policies/darwin/en-us/com.visualstudio.code.oss.plist | Added macOS policy entries for English locale |
| build/lib/test/fixtures/policies/darwin/com.visualstudio.code.oss.mobileconfig | Added mobile config policy entries for macOS |
| build/lib/policies/policyData.jsonc | Added comprehensive policy data with full default auto-approve rules (110+ lines of command definitions) |
| <key>pfm_type</key> | ||
| <string>string</string> |
There was a problem hiding this comment.
Duplicate pfm_type key detected. The key 'pfm_type' appears twice in the same dictionary entry for ChatToolsTerminalAutoApprove. This will cause the second declaration to override the first, making line 225 redundant. Remove the duplicate declaration on line 226.
| <key>pfm_type</key> | |
| <string>string</string> |
| localization: { | ||
| description: { | ||
| key: 'ignoreDefaultAutoApproveRules.policy.description', | ||
| value: localize('ignoreDefaultAutoApproveRules.policy.description', "Whether to ignore the built-in default auto-approve rules used by the run in terminal tool."), |
There was a problem hiding this comment.
The 'experimental' tag was removed from this setting, which promotes it to a stable feature. However, the policy description has been simplified compared to the more detailed user-facing description. Consider whether administrators need more context about the security implications of this setting, similar to how the user-facing description warns "Use this setting at your own risk; the default auto-approve rules are designed to protect you against running dangerous commands."
| value: localize('ignoreDefaultAutoApproveRules.policy.description', "Whether to ignore the built-in default auto-approve rules used by the run in terminal tool."), | |
| value: localize('ignoreDefaultAutoApproveRules.policy.description', "Whether to ignore the built-in default auto-approve rules used by the run in terminal tool as defined in {0}. When this setting is enabled, the run in terminal tool will ignore any rule that comes from the default set but still follow rules defined in the user, remote and workspace settings. Use this setting at your own risk; the default auto-approve rules are designed to protect you against running dangerous commands.", `#${TerminalChatAgentToolsSettingId.AutoApprove}#`), |
|
|
||
| Cette fonctionnalité désactive [les protections de sécurité critiques](https://code.visualstudio.com/docs/copilot/security) et facilite considérablement la compromission de la machine par un attaquant.</string> | ||
| <string id="ChatToolsEligibleForAutoApproval">ChatToolsEligibleForAutoApproval</string> | ||
| <string id="ChatToolsEligibleForAutoApproval_chat_tools_eligibleForAutoApproval">Controls which tools are eligible for automatic approval. Tools set to 'false' will always present a confirmation and will never offer the option to auto-approve. The default behavior (or setting a tool to 'true') may result in the tool offering auto-approval options.</string> |
There was a problem hiding this comment.
The description for ChatToolsEligibleForAutoApproval is in English in the French localization file. This string should be translated to French to maintain consistency with the rest of the French localization file (fr-fr).
| <string id="ChatToolsEligibleForAutoApproval_chat_tools_eligibleForAutoApproval">Controls which tools are eligible for automatic approval. Tools set to 'false' will always present a confirmation and will never offer the option to auto-approve. The default behavior (or setting a tool to 'true') may result in the tool offering auto-approval options.</string> | |
| <string id="ChatToolsEligibleForAutoApproval_chat_tools_eligibleForAutoApproval">Contrôle quels outils sont éligibles à l’approbation automatique. Les outils définis sur 'false' demanderont toujours une confirmation et ne proposeront jamais l’option d’approbation automatique. Le comportement par défaut (ou la définition d’un outil sur 'true') peut amener l’outil à proposer des options d’approbation automatique.</string> |
| <string id="ChatToolsTerminalEnableAutoApprove">ChatToolsTerminalEnableAutoApprove</string> | ||
| <string id="ChatToolsTerminalEnableAutoApprove_autoApproveMode_description">Contrôle s’il faut autoriser l’approbation automatique lors de l’exécution dans l’outil terminal.</string> | ||
| <string id="ChatToolsTerminalIgnoreDefaultAutoApproveRules">ChatToolsTerminalIgnoreDefaultAutoApproveRules</string> | ||
| <string id="ChatToolsTerminalIgnoreDefaultAutoApproveRules_ignoreDefaultAutoApproveRules_policy_description">Whether to ignore the built-in default auto-approve rules used by the run in terminal tool.</string> |
There was a problem hiding this comment.
The description for ChatToolsTerminalIgnoreDefaultAutoApproveRules is in English in the French localization file. This string should be translated to French to maintain consistency with the rest of the French localization file (fr-fr).
| <string id="ChatToolsTerminalIgnoreDefaultAutoApproveRules_ignoreDefaultAutoApproveRules_policy_description">Whether to ignore the built-in default auto-approve rules used by the run in terminal tool.</string> | |
| <string id="ChatToolsTerminalIgnoreDefaultAutoApproveRules_ignoreDefaultAutoApproveRules_policy_description">Détermine s’il faut ignorer les règles d’approbation automatique par défaut intégrées utilisées par l’outil d’exécution dans le terminal.</string> |
| <key>pfm_default</key> | ||
| <string></string> | ||
| <key>pfm_description</key> | ||
| <string>A list of commands or regular expressions that control whether the run in terminal tool commands require explicit approval.</string> |
There was a problem hiding this comment.
The description for ChatToolsTerminalAutoApprove is in English in the French localization file. This string should be translated to French to maintain consistency with the rest of the French localization file (fr-fr).
| <string>A list of commands or regular expressions that control whether the run in terminal tool commands require explicit approval.</string> | |
| <string>Une liste de commandes ou d’expressions régulières qui déterminent si les commandes d’exécution dans l’outil terminal nécessitent une approbation explicite.</string> |
| <false/> | ||
| <key>pfm_description</key> | ||
| <string>Activez le mode Assistant pour la conversation. Lorsque cette option est activée, le mode Assistant peut être activé via la liste déroulante de la vue.</string> | ||
| <string>When enabled, agent mode can be activated from chat and tools in agentic contexts with side effects can be used. |
There was a problem hiding this comment.
Missing closing tag for the pfm_description string element. The string starting on line 125 is never closed before the next pfm_name key begins on line 126. Add a closing '' tag at the end of line 125.
| <string>When enabled, agent mode can be activated from chat and tools in agentic contexts with side effects can be used. | |
| <string>When enabled, agent mode can be activated from chat and tools in agentic contexts with side effects can be used.</string> |
| <string id="ExtensionGalleryServiceUrl">ExtensionGalleryServiceUrl</string> | ||
| <string id="ExtensionGalleryServiceUrl_extensions_gallery_serviceUrl">Configurer l’URL du service Place de marché à laquelle se connecter</string> | ||
| <string id="ChatToolsTerminalAutoApprove">ChatToolsTerminalAutoApprove</string> | ||
| <string id="ChatToolsTerminalAutoApprove_autoApprove_policy_description">A list of commands or regular expressions that control whether the run in terminal tool commands require explicit approval.</string> |
There was a problem hiding this comment.
The description for ChatToolsTerminalAutoApprove is in English in the French localization file. This string should be translated to French to maintain consistency with the rest of the French localization file (fr-fr).
| <string id="ChatToolsTerminalAutoApprove_autoApprove_policy_description">A list of commands or regular expressions that control whether the run in terminal tool commands require explicit approval.</string> | |
| <string id="ChatToolsTerminalAutoApprove_autoApprove_policy_description">Liste de commandes ou d’expressions régulières qui contrôlent si les commandes de l’outil Exécuter dans le terminal nécessitent une approbation explicite.</string> |
| <key>pfm_default</key> | ||
| <string></string> | ||
| <key>pfm_description</key> | ||
| <string>Controls which tools are eligible for automatic approval. Tools set to 'false' will always present a confirmation and will never offer the option to auto-approve. The default behavior (or setting a tool to 'true') may result in the tool offering auto-approval options.</string> |
There was a problem hiding this comment.
The description for ChatToolsEligibleForAutoApproval is in English in the French localization file. This string should be translated to French to maintain consistency with the rest of the French localization file (fr-fr).
| <string>Controls which tools are eligible for automatic approval. Tools set to 'false' will always present a confirmation and will never offer the option to auto-approve. The default behavior (or setting a tool to 'true') may result in the tool offering auto-approval options.</string> | |
| <string>Contrôle quels outils sont éligibles à l’approbation automatique. Les outils définis sur 'false' afficheront toujours une demande de confirmation et ne proposeront jamais l’option d’approbation automatique. Le comportement par défaut (ou définir un outil sur 'true') peut amener l’outil à proposer des options d’approbation automatique.</string> |
| <string id="ChatAgentExtensionTools_chat_extensionToolsEnabled">Enable using tools contributed by third-party extensions.</string> | ||
| <string id="ChatAgentMode">ChatAgentMode</string> | ||
| <string id="ChatAgentMode_chat_agent_enabled_description">Enable agent mode for chat. When this is enabled, agent mode can be activated via the dropdown in the view.</string> | ||
| <string id="ChatAgentMode_chat_agent_enabled_description">When enabled, agent mode can be activated from chat and tools in agentic contexts with side effects can be used.</string> |
There was a problem hiding this comment.
Grammatical error in the description. The phrase "tools in agentic contexts with side effects can be used" is awkwardly structured. Consider revising to: "When enabled, agent mode can be activated from chat and tools with side effects can be used in agentic contexts." or "When enabled, agent mode can be activated from chat, allowing tools in agentic contexts with side effects to be used."
| <string id="ChatAgentMode_chat_agent_enabled_description">When enabled, agent mode can be activated from chat and tools in agentic contexts with side effects can be used.</string> | |
| <string id="ChatAgentMode_chat_agent_enabled_description">When enabled, agent mode can be activated from chat and tools with side effects can be used in agentic contexts.</string> |
| description: { | ||
| key: 'chat.agent.enabled.description', | ||
| value: 'Enable agent mode for chat. When this is enabled, agent mode can be activated via the dropdown in the view.' | ||
| value: 'When enabled, agent mode can be activated from chat and tools in agentic contexts with side effects can be used.' |
There was a problem hiding this comment.
Grammatical error in the description. The phrase "tools in agentic contexts with side effects can be used" is awkwardly structured. Consider revising to: "When enabled, agent mode can be activated from chat and tools with side effects can be used in agentic contexts." or "When enabled, agent mode can be activated from chat, allowing tools in agentic contexts with side effects to be used."
| value: 'When enabled, agent mode can be activated from chat and tools in agentic contexts with side effects can be used.' | |
| value: 'When enabled, agent mode can be activated from chat and tools with side effects can be used in agentic contexts.' |
| <key>pfm_type</key> | ||
| <string>boolean</string> | ||
| </dict><dict> | ||
| <key>pfm_default</key> |
There was a problem hiding this comment.
Just a heads up, the goal of the fixtures here is mostly to validate the generator behaviour on the data prior to refactor (and not necessarily the live version of the policy data; otherwise we could've just checked those in as well). Last time @joshspicer ran into this as well when asking Copilot to add a policy, we may need instructions to avoid having it modify it like this.
There was a problem hiding this comment.
Not sure I understand, I was getting these failures: https://github.com/microsoft/vscode/actions/runs/20678051529/job/59368384249
I thought it would work like regenerating the files so they can be reviews carefully in PR?
There was a problem hiding this comment.
The only file that needs to be regenerated and reviewed is policyData.jsonc, which is then converted in product build pipeline into the OS-specific formats (the tests here are to make sure the conversions are as expected). I decided against checking in the OS-specific formats directly since it will bloat PR diffs and we need to regenerate those files anyways to account for product.json values and translations, so having it generated and checked-in could be misleading.
There was a problem hiding this comment.
that being said, if we feel like the policy being added is different from existing types (e.g., new format) then adding tests like this is the right thing to do
Fixes #284302
@joshspicer thoughts on these policies? There is also
chat.tools.terminal.autoReplyToPrompts(experimental) andchat.tools.terminal.blockDetectedFileWrites(experimental) which are potentially risky. It's not clear to me how defaults work when experimental features are disabled as the "safest" value forblockDetectedFileWritesis not the default.