Weekly Permissions sync 2025-12-14 #1379
Conversation
There was a problem hiding this comment.
Pull request overview
This is a weekly permissions sync that updates permission configurations for Copilot-related features. The PR introduces two new permissions for managing Copilot packages and updates the provisioning information for existing permissions.
Key changes:
- Adds two new permissions:
CopilotPackages.Read.AllandCopilotPackages.ReadWrite.Allfor reading and managing Copilot package information - Populates previously empty permission IDs in provisioning info for the new Copilot package permissions
- Updates visibility and enablement flags for Copilot package permissions (making DelegatedWork schemes visible and enabled)
- Changes environment configuration for
User-OnPremisesSyncBehavior.ReadWrite.Allpermission
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| permissions/new/provisioningInfo.json | Adds permission IDs for CopilotPackages permissions, updates visibility/enablement flags, and modifies environment field for User-OnPremisesSyncBehavior permission |
| permissions/new/permissions.json | Defines two new Copilot package permissions with their authorization types, schemes, HTTP methods, API paths, and ownership information |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "schemes": { | ||
| "DelegatedWork": { | ||
| "adminDisplayName": "Read and update all packages information", | ||
| "adminDescription": "Allows the user to read and update the packages information", |
There was a problem hiding this comment.
The description "Allows the user to read and update the packages information" is grammatically awkward. Consider using either "package information" (singular, more idiomatic) or "packages' information" (possessive plural) for better clarity and consistency with other permission descriptions.
| "adminDisplayName": "Read and update all packages information", | ||
| "adminDescription": "Allows the user to read and update the packages information", | ||
| "userDisplayName": "Read and update all packages information", | ||
| "userDescription": "Allows the app to read and update packages information.", |
There was a problem hiding this comment.
The description "Allows the app to read and update packages information." is grammatically awkward. Consider using either "package information" (singular, more idiomatic) or "packages' information" (possessive plural) for better clarity and consistency with other permission descriptions.
| }, | ||
| "Application": { | ||
| "adminDisplayName": "Read and update all packages information", | ||
| "adminDescription": "Allows the app to read and update packages information without a signed-in user.", |
There was a problem hiding this comment.
The description "Allows the app to read and update packages information without a signed-in user." is grammatically awkward. Consider using either "package information" (singular, more idiomatic) or "packages' information" (possessive plural) for better clarity and consistency with other permission descriptions.
| "methods": [ | ||
| "GET", | ||
| "POST" |
There was a problem hiding this comment.
The CopilotPackages.ReadWrite.All permission claims to allow "read and update" operations but only specifies GET and POST methods. Typically, update operations in REST APIs use PATCH or PUT methods. Consider whether PATCH or PUT should be included in the methods array to properly support update operations, or if the permission description should be updated to clarify that only creation (POST) is supported alongside reading (GET).
| "id": "7ff9afdd-0cdb-439d-a61c-fea3e9339e89", | ||
| "scheme": "DelegatedWork", | ||
| "environment": "PPE;public", | ||
| "environment": "", |
There was a problem hiding this comment.
The environment field is being changed from "PPE;public" to an empty string for the DelegatedWork scheme, but the Application scheme on line 15983 retains "PPE;public". This creates an inconsistency within the same permission "User-OnPremisesSyncBehavior.ReadWrite.All" where the two schemes have different environment values. Consider whether both schemes should have the same environment value for consistency.
| "environment": "", | |
| "environment": "PPE;public", |
| "schemes": { | ||
| "DelegatedWork": { | ||
| "adminDisplayName": "Read all packages information", | ||
| "adminDescription": "Allows the user to read the packages information", |
There was a problem hiding this comment.
The description "Allows the user to read the packages information" is grammatically awkward. Consider using either "package information" (singular, more idiomatic) or "packages' information" (possessive plural) for better clarity and consistency with other permission descriptions.
| "adminDisplayName": "Read all packages information", | ||
| "adminDescription": "Allows the user to read the packages information", | ||
| "userDisplayName": "Read all packages information", | ||
| "userDescription": "Allows the app to read packages information.", |
There was a problem hiding this comment.
The description "Allows the app to read packages information." is grammatically awkward. Consider using either "package information" (singular, more idiomatic) or "packages' information" (possessive plural) for better clarity and consistency with other permission descriptions.
| }, | ||
| "Application": { | ||
| "adminDisplayName": "Read all packages information", | ||
| "adminDescription": "Allows the app to read packages information without a signed-in user.", |
There was a problem hiding this comment.
The description "Allows the app to read packages information without a signed-in user." is grammatically awkward. Consider using either "package information" (singular, more idiomatic) or "packages' information" (possessive plural) for better clarity and consistency with other permission descriptions.
Weekly Permissions sync 2025-12-14