Conversation
Setting minimum version for Graph Auth/Teams to 2.35.1 Adding Clear-ZtRequiredModules for cleanup
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes the build system by replacing the broken PowerShellGet/Update-ModuleManifest cmdlet with the Update-Metadata cmdlet from @Jaykul's Metadata module. This change is necessary to properly update module manifest properties during the build process without corrupting custom PrivateData keys. The PR also updates Microsoft Graph module dependencies, adds a new cleanup function, and changes the default service connection behavior.
Changes:
- Replaced
Update-ModuleManifestwithUpdate-Metadatathroughout build scripts to avoid manifest corruption issues - Updated Microsoft.Graph.Authentication and Microsoft.Graph.Beta.Teams minimum versions to 2.35.0
- Added
Clear-ZtRequiredModulefunction to cleanup downloaded module dependencies - Changed default
Serviceparameter from 'Graph' to 'All' inConnect-ZtAssessment - Refactored dependency resolution logic to use Find-PSResource before Save-PSResource for better version control
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/powershell/public/Connect-ZtAssessment.ps1 | Updated documentation and changed default Service parameter to 'All'; updated Graph module versions |
| src/powershell/public/Clear-ZtRequiredModule.ps1 | New function to remove downloaded module dependencies from cache directories |
| src/powershell/ZeroTrustAssessment.psd1 | Updated Graph module versions in manifest and added Clear-ZtRequiredModule to exports |
| src/powershell/Initialize-Dependencies.ps1 | Refactored module dependency resolution with Find-PSResource approach |
| build/powershell/Update-PSModuleManifest.ps1 | Replaced Update-ModuleManifest with Update-Metadata for safer manifest updates |
| build/powershell/Set-Version.ps1 | Replaced Update-ModuleManifest with Update-Metadata |
| build/powershell/Resolve-PSDependencies.ps1 | Refactored module dependency resolution logic |
| build/powershell/Install-Prerequisites.ps1 | Added Metadata module as build prerequisite |
| build/common-functions.ps1 | Replaced Update-ModuleManifest with Update-Metadata and cleaned up trailing whitespace |
| build/commands/Set-TestMetadata.ps1 | Fixed parameter indentation formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @{ModuleName = 'Microsoft.Graph.Authentication'; GUID = '883916f2-9184-46ee-b1f8-b6a2fb784cee'; ModuleVersion = '2.35.0'; }, | ||
| @{ModuleName = 'Microsoft.Graph.Beta.Teams'; GUID = 'e264919d-7ae2-4a89-ba8b-524bd93ddc08'; ModuleVersion = '2.35.0'; }, |
There was a problem hiding this comment.
| if ($isWindows -and (Get-Command -Name Set-Clipboard -ErrorAction SilentlyContinue)) { | ||
| Set-Clipboard -Value ('&''{0}''' -f $PSCommandPath) | ||
| Write-Warning -Message '(The command has been copied to your clipboard.)' | ||
| } | ||
| return | ||
| } | ||
| else | ||
| { | ||
| Write-Verbose -Message 'Clearing ZTA required modules from the current session.' | ||
| } | ||
|
|
||
| # Remove all ZTA-related modules from the current session | ||
| if ($isWindows) { |
There was a problem hiding this comment.
The variable name should be $IsWindows (capital I) to match the PowerShell automatic variable and be consistent with the rest of the codebase. The lowercase $isWindows will cause this code to fail as the variable is undefined.
| } | ||
| elseif ($moduleSpec.Version) { | ||
| # Minimum version inclusive | ||
| $findModuleParams['Version'] = '[{0}, )' -f $moduleSpec.Version |
There was a problem hiding this comment.
There's inconsistent indentation here - this line has an extra space at the beginning compared to the other similar lines (106-116). Should align with the other conditional branches for consistency.
| $findModuleParams['Version'] = '[{0}, )' -f $moduleSpec.Version | |
| $findModuleParams['Version'] = '[{0}, )' -f $moduleSpec.Version |
| Force = $true | ||
| Path = $RequiredModulesPath | ||
| } | ||
|
|
There was a problem hiding this comment.
When using Save-Module (as opposed to Save-PSResource), the AllowPrerelease parameter should be passed if $AllowPrerelease.IsPresent is true, otherwise prerelease versions won't be saved when requested. Add the AllowPrerelease parameter to $saveModuleCmdParams conditionally based on the $AllowPrerelease parameter.
| if ($AllowPrerelease.IsPresent) | |
| { | |
| $saveModuleCmdParams['AllowPrerelease'] = $true | |
| } |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
build/powershell/Update-PSModuleManifest.ps1:75
- The
Install-Modulecall here fetches and installs required modules from the publicPSGallerywith-Force,-SkipPublisherCheck, and no explicit version pinning, which creates a strong supply-chain risk. If an attacker compromises or typosquats a required module on PSGallery, this build script will silently install and later execute that malicious code with your build/developer privileges. To mitigate this, pin modules to specific versions (or verified publisher/signature), avoid-SkipPublisherCheckfor production/build automation, and consider using a private/curated repository instead of the public gallery for build-time dependencies.
## Install Module Dependencies
foreach ($Module in $ModuleManifest['RequiredModules']) {
if ($Module -is [hashtable]) { $ModuleName = $Module.ModuleName }
else { $ModuleName = $Module }
if ($ModuleName -notin $ModuleManifest.PrivateData.PSData['ExternalModuleDependencies'] -and !(Get-Module $ModuleName -ListAvailable)) {
Install-Module $ModuleName -Force -SkipPublisherCheck -Repository PSGallery -AcceptLicense
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Write-Warning -Message 'This command cannot be run when the module is loaded.' | ||
| Write-Warning -Message 'Please close all sessions where ZeroTrustAssessment module is loaded, then run the following...' | ||
| Write-Warning -Message ('&''{0}''' -f $PSCommandPath) | ||
| if ($isWindows -and (Get-Command -Name Set-Clipboard -ErrorAction SilentlyContinue)) { |
There was a problem hiding this comment.
Incorrect casing for the automatic variable. Should use $IsWindows (with capital 'I') instead of $isWindows to match PowerShell's automatic variable naming and be consistent with the rest of the codebase.
| # if ($paramUpdateModuleManifest.ContainsKey('RequiredAssemblies')) { | ||
| # if (!$paramUpdateModuleManifest['RequiredAssemblies']) { $paramUpdateModuleManifest.Remove('RequiredAssemblies') } | ||
| # (Get-Content $ModuleManifestFileInfo.FullName -Raw) -replace "(?s)(#\s*)?RequiredAssemblies\s*=\s*@\([^)]*\)", "# RequiredAssemblies = @()" | Set-Content $ModuleManifestFileInfo.FullName | ||
| # } | ||
| # if ($paramUpdateModuleManifest.ContainsKey('NestedModules') -and !$paramUpdateModuleManifest['NestedModules']) { | ||
| # $paramUpdateModuleManifest.Remove('NestedModules') | ||
| # (Get-Content $ModuleManifestFileInfo.FullName -Raw) -replace "(?s)(#\s*)?NestedModules\s*=\s*@\([^)]*\)", "# NestedModules = @()" | Set-Content $ModuleManifestFileInfo.FullName | ||
| # } | ||
| # if ($paramUpdateModuleManifest.ContainsKey('FileList')) { | ||
| # (Get-Content $ModuleManifestFileInfo.FullName -Raw) -replace "(?s)(#\s*)?FileList\s*=\s*@\([^)]*\)", "# FileList = @()" | Set-Content $ModuleManifestFileInfo.FullName | ||
| # } |
There was a problem hiding this comment.
Consider removing the commented-out code instead of leaving it in the file. Dead code can cause confusion and reduce maintainability. If the old implementation needs to be referenced, it's available in version control history.
@merill : This PR is only to be merged after #934 (will rebase once merged and set to ready)
This fix replaces the broken
PowerShellGet/Update-ModuleManifestwith @Jaykul'sUpdate-Metadata(from his Metadata module) to update the ModuleManifest during build (RequiredAssemblies, NestedModules, FileList, ModuleVersion, FunctionsToExport, Prerelease...)