From 18680edd97a7b7061355576a565ca961e0c1e758 Mon Sep 17 00:00:00 2001 From: Roy Klooster Date: Mon, 24 Aug 2026 12:58:17 +0200 Subject: [PATCH 01/20] fix: Invoke-InforcerAssessment could never reach the API Every call failed, single and multi-tenant, with 400 ValidationFailure "Unspecified content type application/json is not allowed". POST /beta/tenants/{id}/assessments/{id}/runs takes no request body and rejects ANY Content-Type. Verified with curl: no header returns 200, application/json and text/plain both 400, and the error echoes back whichever type was sent. Removing the header from the hashtable is not enough - Invoke-RestMethod supplies application/x-www-form-urlencoded on a bodyless POST, which is rejected the same way. -ContentType '' suppresses it entirely. Also guards the zero-result case. When every tenant failed, the run loop fell through to renderers whose -TenantResults is mandatory, so the user got "Cannot bind argument to parameter 'TenantResults' because it is an empty collection" - a parameter-binding error that says nothing about why the runs failed. One guard after the loop covers the Json, Html and Csv paths. Verified against api-uk with two tenants: 21 checks each, matrix HTML written, 76.2% and 71.4%. Re-breaking only the Content-Type confirms the guard now reports NoAssessmentResults and writes no file. 399 tests pass. --- module/Private/Invoke-InforcerAssessmentRun.ps1 | 8 ++++++-- module/Public/Invoke-InforcerAssessment.ps1 | 9 +++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/module/Private/Invoke-InforcerAssessmentRun.ps1 b/module/Private/Invoke-InforcerAssessmentRun.ps1 index c6893bb..392fc4a 100644 --- a/module/Private/Invoke-InforcerAssessmentRun.ps1 +++ b/module/Private/Invoke-InforcerAssessmentRun.ps1 @@ -30,10 +30,14 @@ function Invoke-InforcerAssessmentRun { $endpoint = "/beta/tenants/$ClientTenantId/assessments/$ResolvedAssessmentId/runs" $uri = $script:InforcerSession.BaseUrl + $endpoint $apiKey = ConvertFrom-InforcerSecureString -SecureString $script:InforcerSession.ApiKey - $headers = @{ 'Inf-Api-Key' = $apiKey; 'Content-Type' = 'application/json' } + $headers = @{ 'Inf-Api-Key' = $apiKey } + # This endpoint takes no request body and rejects ANY Content-Type with + # 400 ValidationFailure ("Unspecified content type is not allowed"). Omitting the + # header is not enough: Invoke-RestMethod supplies application/x-www-form-urlencoded on a + # bodyless POST, which is rejected the same way. -ContentType '' suppresses it entirely. $ps = [PowerShell]::Create() - $null = $ps.AddScript('param($Uri, $Headers); Invoke-RestMethod -Uri $Uri -Method POST -Headers $Headers') + $null = $ps.AddScript('param($Uri, $Headers); Invoke-RestMethod -Uri $Uri -Method POST -Headers $Headers -ContentType ""') $null = $ps.AddParameter('Uri', $uri) $null = $ps.AddParameter('Headers', $headers) $asyncResult = $ps.BeginInvoke() diff --git a/module/Public/Invoke-InforcerAssessment.ps1 b/module/Public/Invoke-InforcerAssessment.ps1 index ebe705d..5c930d8 100644 --- a/module/Public/Invoke-InforcerAssessment.ps1 +++ b/module/Public/Invoke-InforcerAssessment.ps1 @@ -274,6 +274,15 @@ if ($isMultiTenant) { Write-Host "" Write-Host "All assessments complete. $($allTenantResults.Count) tenant(s) processed in $totalTimeStr." -ForegroundColor Green + # Every tenant failed. Bail before the summary and every export path: the HTML and CSV + # renderers take a mandatory -TenantResults, so falling through binds an empty collection + # and throws a parameter-binding error that says nothing about why the runs failed. + # One guard here covers the Json, Html and Csv paths below. + if ($allTenantResults.Count -eq 0) { + Write-Error -Message 'No assessment results were returned for any tenant, so there is nothing to report. See the errors above for the per-tenant failures.' -ErrorId 'NoAssessmentResults' -Category InvalidResult + return + } + # Summary per tenant foreach ($tr in $allTenantResults) { $color = if ($tr.Score -ge 90) { 'Green' } elseif ($tr.Score -ge 70) { 'Yellow' } else { 'Red' } From 24e84bc415cdea0e9398b3478e517ddf1a009d16 Mon Sep 17 00:00:00 2001 From: Roy Klooster Date: Mon, 24 Aug 2026 13:28:56 +0200 Subject: [PATCH 02/20] fix: one-sided baseline scoping reported a meaningless alignment score -SourceBaselineId scoped the source and left the destination at its full policy set, so N baseline policies were compared against the destination's entire estate and every destination-only policy counted as a deviation: -SourceBaselineId 'demo - partial baseline' -DestinationTenantId 18159 before: 0.2% over 1422 items (3 source policies vs 756 destination) after: 100% over 3 items (3 vs 3, matches Inforcer's own score) The docstring advertised exactly that one-sided form as an example, so the documented usage produced the misleading number. The destination now inherits -SourceBaselineId unless -DestinationBaselineId overrides it. When the destination cannot be scoped - it may legitimately not be a member of the source's baseline - it falls back to the full policy set with a warning naming the consequence, rather than erroring. An explicitly passed -DestinationBaselineId that fails is still an error. Safe because Select-InforcerBaselinePolicies returns null before it mutates DocData, so the unfiltered destination survives intact, and DestinationBaselineName stays null so neither the report header nor the filename claims a scope that was not applied. Also: -Tag matching nothing rendered an empty document and exited 0, which reads as 'nothing here is tagged that way' when the likelier cause is a tag name that does not exist. It now errors with the tags that do exist: No policy matched tag 'Tier 1', so there is nothing to document. Tags present in this tenant: 'Low Impact', 'MacOS Baseline - Core', ..., 'Tier 0'. Both verified against api-uk, including the fallback branch via a forced destination-scope failure. 399 tests pass; analyzer counts unchanged on all three files. --- module/Private/Get-InforcerComparisonData.ps1 | 34 +++++++++++++++---- .../Public/Compare-InforcerEnvironments.ps1 | 9 ++++- .../Export-InforcerTenantDocumentation.ps1 | 22 ++++++++++++ 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/module/Private/Get-InforcerComparisonData.ps1 b/module/Private/Get-InforcerComparisonData.ps1 index ae2c66d..28b821e 100644 --- a/module/Private/Get-InforcerComparisonData.ps1 +++ b/module/Private/Get-InforcerComparisonData.ps1 @@ -108,13 +108,35 @@ function Get-InforcerComparisonData { } # ── Destination baseline filtering (while dest session is active) ── - if (-not [string]::IsNullOrWhiteSpace($DestinationBaselineId)) { - Write-Host " Filtering destination to baseline: $DestinationBaselineId" -ForegroundColor Gray - $destBaselineName = Select-InforcerBaselinePolicies -DocData $destDocData -BaselineId $DestinationBaselineId + # The destination inherits the source's baseline when none was given. Scoping one side + # only compares N baseline policies against the destination's ENTIRE estate, so every + # destination-only policy scores as a deviation: 3-policy baseline vs 756-policy tenant + # reported 0.2% where Inforcer itself reports 100% for the same pair. "How aligned is + # this tenant to this baseline" means comparing like with like on both sides. + $effectiveDestBaseline = $DestinationBaselineId + $baselineInherited = $false + if ([string]::IsNullOrWhiteSpace($effectiveDestBaseline) -and -not [string]::IsNullOrWhiteSpace($SourceBaselineId)) { + $effectiveDestBaseline = $SourceBaselineId + $baselineInherited = $true + } + + if (-not [string]::IsNullOrWhiteSpace($effectiveDestBaseline)) { + Write-Host " Filtering destination to baseline: $effectiveDestBaseline" -ForegroundColor Gray + $destBaselineName = Select-InforcerBaselinePolicies -DocData $destDocData -BaselineId $effectiveDestBaseline if ($null -eq $destBaselineName) { - Write-Error -Message "Failed to filter destination tenant to baseline '$DestinationBaselineId'." ` - -ErrorId 'DestBaselineFilterFailed' -Category InvalidResult - return $null + # An explicit -DestinationBaselineId that fails is an error. An inherited one is + # not: the destination simply may not be a member of the source's baseline, which + # is a legitimate comparison to want. Select-InforcerBaselinePolicies returns + # $null before it mutates DocData, so the unfiltered destination is still intact. + if (-not $baselineInherited) { + Write-Error -Message "Failed to filter destination tenant to baseline '$DestinationBaselineId'." ` + -ErrorId 'DestBaselineFilterFailed' -Category InvalidResult + return $null + } + Write-Warning ("Destination tenant '$DestinationTenantId' could not be scoped to baseline '$SourceBaselineId' " + + 'so the comparison runs against its full policy set. Every destination-only policy then counts as a ' + + 'deviation, which understates the alignment score. Pass -DestinationBaselineId to scope it explicitly.') + $effectiveDestBaseline = $null } } } finally { diff --git a/module/Public/Compare-InforcerEnvironments.ps1 b/module/Public/Compare-InforcerEnvironments.ps1 index 2c5da6d..285866c 100644 --- a/module/Public/Compare-InforcerEnvironments.ps1 +++ b/module/Public/Compare-InforcerEnvironments.ps1 @@ -26,9 +26,16 @@ .PARAMETER SourceBaselineId Optional baseline GUID or friendly name for the source tenant. When specified, the comparison is scoped to only policies belonging to this baseline instead of all tenant policies. + + The destination is scoped to the same baseline unless -DestinationBaselineId says otherwise. + Scoping one side only would compare a handful of baseline policies against the destination's + entire estate, counting every destination-only policy as a deviation. If the destination + tenant is not a member of the baseline it cannot be scoped, and the comparison falls back to + its full policy set with a warning. .PARAMETER DestinationBaselineId Optional baseline GUID or friendly name for the destination tenant. When specified, the comparison - is scoped to only policies belonging to this baseline instead of all tenant policies. + is scoped to only policies belonging to this baseline instead of all tenant policies. Defaults + to -SourceBaselineId when that is given; pass it explicitly to compare across two baselines. .PARAMETER IncludingAssignments When specified, fetches and displays Graph assignment data in the report. Assignments are informational only and do not affect the alignment score. diff --git a/module/Public/Export-InforcerTenantDocumentation.ps1 b/module/Public/Export-InforcerTenantDocumentation.ps1 index 3b6bd1e..9533cf8 100644 --- a/module/Public/Export-InforcerTenantDocumentation.ps1 +++ b/module/Public/Export-InforcerTenantDocumentation.ps1 @@ -141,6 +141,17 @@ if (-not [string]::IsNullOrWhiteSpace($Baseline)) { if (-not [string]::IsNullOrWhiteSpace($Tag)) { Write-Host "Filtering to tag: $Tag" -ForegroundColor Cyan $originalCount = @($docData.Policies).Count + + # Collected before the filter so a no-match can name the tags that do exist. + $availableTags = [System.Collections.Generic.SortedSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) + foreach ($pol in @($docData.Policies)) { + foreach ($t in @($pol.tags)) { + if ($null -eq $t) { continue } + $n = if ($t -is [PSObject] -and $t.PSObject.Properties['name']) { $t.name } else { $t.ToString() } + if (-not [string]::IsNullOrWhiteSpace($n)) { [void]$availableTags.Add($n) } + } + } + $docData.Policies = @($docData.Policies | Where-Object { $policyTags = $_.tags if ($null -eq $policyTags -or @($policyTags).Count -eq 0) { return $false } @@ -151,6 +162,17 @@ if (-not [string]::IsNullOrWhiteSpace($Tag)) { return $false }) Write-Host " Filtered to $(@($docData.Policies).Count) of $originalCount policies with tag '$Tag'" -ForegroundColor Gray + + # No match used to render anyway: a 0.1 KB document and exit 0, which reads as "nothing in + # this tenant is tagged that way" when the likelier cause is a tag name that does not exist. + # Naming the real tags is the difference between a dead end and a correctable mistake. + if (@($docData.Policies).Count -eq 0) { + $known = if ($availableTags.Count -gt 0) { $availableTags -join "', '" } else { $null } + $detail = if ($known) { "Tags present in this tenant: '$known'." } else { 'No policy in this tenant carries any tag.' } + Write-Error -Message "No policy matched tag '$Tag', so there is nothing to document. $detail" ` + -ErrorId 'TagMatchedNothing' -Category ObjectNotFound + return + } } # Load Settings Catalog only if there are Settings Catalog policies (policyTypeId 10) From 769669c27b217723bab75e8596effdced1ac9cb4 Mon Sep 17 00:00:00 2001 From: Roy Klooster Date: Mon, 24 Aug 2026 13:55:33 +0200 Subject: [PATCH 03/20] feat!: writing files and opening a browser are both opt-in Compare-InforcerEnvironments and Export-InforcerTenantDocumentation both had `$OutputPath = '.'` and an unconditional Start-Process on the rendered HTML, so neither could run without dropping a file in the caller's working directory and spawning a browser window. In a pipeline or a container that is wrong twice over: the file is litter and there is nothing to open it with. Both now: - write only when -OutputPath is given. No default. Without it the cmdlet returns the model - the comparison hashtable, or the DocModel - so a caller can read the numbers without touching disk. -OutputPath's presence IS the opt-in; a separate -Export switch would carry no information the path does not already carry. - open a browser only with -Show. BREAKING CHANGE: Compare-InforcerEnvironments and Export-InforcerTenantDocumentation no longer write to the current directory by default, and no longer open a browser. Callers relying on either must pass -OutputPath and/or -Show. Both return the model instead of System.IO.FileInfo when -OutputPath is omitted. Separately, -ExcludeOS was a no-op for every value its own documentation gave as an example. It matched only $productName (Entra, Intune, Defender, ...), while the OS lives in the category key built from primaryGroup (Windows, macOS, iOS/iPadOS, Android). 'macOS','iOS' removed 0 of 2229 items while reporting success. It now matches the category key as well as the product name, so the documented values work and passing a product name still does: full 2229 items, 61.1% -ExcludeOS macOS,iOS 1509 items, 72.3% (720 removed, was 0) -ExcludeOS Defender 2111 items, 58.9% (118 removed, unchanged) This also corrects an earlier claim of mine that -ExcludeOS worked: the item drop I attributed to it was the tenant's own data changing between runs. 399 tests pass. Analyzer gains one PSAvoidUsingWriteHost per touched file, matching the module's existing deliberate use of Write-Host for progress. --- module/Private/Compare-InforcerDocModels.ps1 | 15 +++++ .../Public/Compare-InforcerEnvironments.ps1 | 57 ++++++++++++++----- .../Export-InforcerTenantDocumentation.ps1 | 57 +++++++++++++------ 3 files changed, 100 insertions(+), 29 deletions(-) diff --git a/module/Private/Compare-InforcerDocModels.ps1 b/module/Private/Compare-InforcerDocModels.ps1 index df0839b..4cac782 100644 --- a/module/Private/Compare-InforcerDocModels.ps1 +++ b/module/Private/Compare-InforcerDocModels.ps1 @@ -420,6 +420,21 @@ function Compare-InforcerDocModels { # Exclude Custom Indicators category entirely (noise — per-tenant unique data) if ($categoryName -match 'Custom Indicators') { continue } + # -ExcludeOS also applies here, not only to product names. The OS lives in the + # category key (built from primaryGroup: Windows, macOS, iOS/iPadOS, Android), never + # in the product name (Entra, Intune, Defender, ...). Matching products alone made + # every documented example value - 'macOS', 'iOS', 'Android', 'Windows' - a silent + # no-op that removed zero items while reporting success. Products are still matched + # above, so passing a product name keeps working. + if ($ExcludeOS) { + $catLower = $categoryName.ToLowerInvariant() + $skipCategory = $false + foreach ($ep in $ExcludeOS) { + if ($catLower -match [regex]::Escape($ep.ToLowerInvariant())) { $skipCategory = $true; break } + } + if ($skipCategory) { continue } + } + $srcPolicies = @() $dstPolicies = @() if ($srcProduct -and $srcProduct.Categories -and $srcProduct.Categories.Contains($categoryName)) { diff --git a/module/Public/Compare-InforcerEnvironments.ps1 b/module/Public/Compare-InforcerEnvironments.ps1 index 285866c..42ecd71 100644 --- a/module/Public/Compare-InforcerEnvironments.ps1 +++ b/module/Public/Compare-InforcerEnvironments.ps1 @@ -54,19 +54,33 @@ - Scope tag resolution (tag ID to display name) - Compliance rules for custom compliance policies (rulesContent via $expand) .PARAMETER ExcludeOS - Array of OS/platform names to exclude from the comparison. Matching is case-insensitive - and uses contains logic. Examples: 'macOS', 'iOS', 'Android', 'Windows'. + Array of OS/platform or product names to exclude from the comparison. Matching is + case-insensitive contains logic, applied to both the product name (Entra, Intune, Defender, + Exchange, SharePoint) and the category key, which is where the OS actually lives + (Windows, macOS, iOS/iPadOS, Android). Examples: 'macOS', 'iOS', 'Android', 'Windows'. Excluded platforms do not affect the alignment score. .PARAMETER PolicyNameFilter Only include policies whose name contains this string (case-insensitive). Non-matching policies are excluded from both the report and the alignment score. .PARAMETER OutputPath - Directory where the HTML report will be written. Defaults to current directory. + Directory where the HTML report will be written. Omit it and no file is written: the + comparison model is returned instead, so a caller can read the numbers without touching disk. + There is no default - writing is always something you asked for. +.PARAMETER Show + Open the generated HTML in the default browser. Requires -OutputPath. Off by default so the + cmdlet is safe in pipelines and containers, where there is no browser to open. .OUTPUTS - System.IO.FileInfo. Returns a FileInfo object for the exported HTML report. + The comparison model (hashtable) when -OutputPath is omitted, otherwise System.IO.FileInfo for + the exported HTML report. .EXAMPLE Connect-Inforcer -ApiKey $key Compare-InforcerEnvironments -SourceTenantId 'Contoso' -DestinationTenantId 'Fabrikam' + + Returns the comparison model. No file is written and no browser opens. +.EXAMPLE + Compare-InforcerEnvironments -SourceTenantId 'Contoso' -DestinationTenantId 'Fabrikam' -OutputPath ./reports -Show + + Writes the HTML report to ./reports and opens it. .EXAMPLE $src = Connect-Inforcer -ApiKey $key1 -Region uk -PassThru $dst = Connect-Inforcer -ApiKey $key2 -Region eu -PassThru @@ -75,7 +89,8 @@ Compare-InforcerEnvironments -SourceTenantId 482 -DestinationTenantId 139 -IncludingAssignments .EXAMPLE Compare-InforcerEnvironments -SourceTenantId 'Contoso' -SourceBaselineId 'Tier 1 Foundations' -DestinationTenantId 'Fabrikam' - # Compares only policies in the 'Tier 1 Foundations' baseline from Contoso against all Fabrikam policies. + # Compares the 'Tier 1 Foundations' baseline policies on both sides. Fabrikam is scoped to the + # same baseline unless -DestinationBaselineId says otherwise. .EXAMPLE Compare-InforcerEnvironments -SourceBaselineId 'Inforcer Blueprint Baseline - Tier 1 - Foundations' -DestinationTenantId 14506 # Compares the baseline (auto-resolves owner tenant) against a specific tenant. @@ -89,7 +104,7 @@ #> function Compare-InforcerEnvironments { [CmdletBinding()] -[OutputType([System.IO.FileInfo])] +[OutputType([hashtable], [System.IO.FileInfo])] param( [Parameter(Position = 0)] [object]$SourceTenantId, @@ -125,7 +140,10 @@ param( [string]$PolicyNameFilter, [Parameter(Mandatory = $false)] - [string]$OutputPath = '.' + [string]$OutputPath, + + [Parameter(Mandatory = $false)] + [switch]$Show ) # Session guard: require an active session unless both explicit sessions are provided @@ -257,7 +275,7 @@ $compareParams = @{ } if ($ExcludeOS) { $compareParams['ExcludeOS'] = $ExcludeOS - Write-Host " Excluding products: $($ExcludeOS -join ', ')" -ForegroundColor Gray + Write-Host " Excluding products/platforms: $($ExcludeOS -join ', ')" -ForegroundColor Gray } if ($PolicyNameFilter) { $compareParams['PolicyNameFilter'] = $PolicyNameFilter @@ -278,6 +296,16 @@ if ($null -eq $model) { Write-Host " Alignment score: $($model.AlignmentScore)%" -ForegroundColor Gray Write-Host " Total items: $($model.TotalItems)" -ForegroundColor Gray +# ── No -OutputPath: hand back the model and touch nothing ───────────────────── +# Rendering and writing are opt-in. Comparing two tenants is a question, not a request for a +# file, and the old default wrote HTML into the caller's working directory and launched a +# browser for it - wrong in a pipeline, wrong in a container, and wrong for any caller that +# just wanted the numbers. +if ([string]::IsNullOrWhiteSpace($OutputPath)) { + Write-Host "Done. Pass -OutputPath to write an HTML report, -Show to open it." -ForegroundColor Cyan + return $model +} + # ── Stage 3: Render HTML report ─────────────────────────────────────────────── Write-Host 'Stage 3: Rendering HTML report...' -ForegroundColor Cyan @@ -316,11 +344,14 @@ $fileInfo = Get-Item -LiteralPath $filePath $sizeKb = [math]::Round($fileInfo.Length / 1KB, 1) Write-Host " Exported: $filePath ($sizeKb KB)" -ForegroundColor Green -# Auto-open HTML output in the default browser (cross-platform) -$fullPath = (Resolve-Path -LiteralPath $filePath).Path -if ($IsMacOS) { Start-Process 'open' -ArgumentList $fullPath } -elseif ($IsWindows) { Start-Process $fullPath } -elseif ($IsLinux) { Start-Process 'xdg-open' -ArgumentList $fullPath } +# Opening a browser is opt-in via -Show. Doing it unconditionally spawned a window on every +# run, including inside CI containers where there is nothing to open it with. +if ($Show) { + $fullPath = (Resolve-Path -LiteralPath $filePath).Path + if ($IsMacOS) { Start-Process 'open' -ArgumentList $fullPath } + elseif ($IsWindows) { Start-Process $fullPath } + elseif ($IsLinux) { Start-Process 'xdg-open' -ArgumentList $fullPath } +} Write-Host "Done. Comparison report generated for '$($compData.SourceName)' vs '$($compData.DestinationName)'." -ForegroundColor Cyan diff --git a/module/Public/Export-InforcerTenantDocumentation.ps1 b/module/Public/Export-InforcerTenantDocumentation.ps1 index 9533cf8..249729b 100644 --- a/module/Public/Export-InforcerTenantDocumentation.ps1 +++ b/module/Public/Export-InforcerTenantDocumentation.ps1 @@ -18,7 +18,8 @@ Before calling this cmdlet, you must be connected via Connect-Inforcer. If no active session exists, the cmdlet emits a non-terminating error and returns immediately. - Output files are written to the specified OutputPath directory and auto-named as + Nothing is written unless -OutputPath is given: without it the DocModel is returned so a + caller can read the configuration without producing files. With it, output is auto-named {TenantName}-Documentation.{ext} (e.g., Contoso-Documentation.html). .PARAMETER Format Output format(s) to generate. Accepted values: Html, Markdown, Excel. Multiple formats @@ -29,7 +30,11 @@ .PARAMETER OutputPath Directory to write output files to. Files are auto-named {TenantName}-Documentation.{ext}. When a single format is specified and this path has a file extension, it is treated as an - explicit output file path. Defaults to the current directory. + explicit output file path. Omit it and no files are written: the DocModel is returned instead. + There is no default - writing is always something you asked for. +.PARAMETER Show + Open the generated HTML in the default browser. Requires -OutputPath and -Format Html. Off by + default so the cmdlet is safe in pipelines and containers, where there is no browser to open. .PARAMETER SettingsCatalogPath Path to a local settings.json file for Settings Catalog resolution. When omitted, the cmdlet automatically downloads and caches the latest data from the IntuneSettingsCatalogData GitHub @@ -51,11 +56,16 @@ Filter to only policies that have a specific Inforcer tag (e.g., "IAM - Core", "Tier 1"). Matches against the tag name property on each policy (case-insensitive, contains match). .OUTPUTS - System.IO.FileInfo. Returns FileInfo objects for each exported file. + The DocModel (hashtable) when -OutputPath is omitted, otherwise System.IO.FileInfo for each + exported file. .EXAMPLE Export-InforcerTenantDocumentation -TenantId 482 -Format Html - Writes Contoso-Documentation.html to the current directory. + Returns the DocModel. No file is written - pass -OutputPath for that. +.EXAMPLE + Export-InforcerTenantDocumentation -TenantId 482 -Format Html -OutputPath ./docs -Show + + Writes Contoso-Documentation.html to ./docs and opens it. .EXAMPLE Export-InforcerTenantDocumentation -TenantId 482 -Format Html,Markdown,Excel -OutputPath C:\Reports @@ -75,7 +85,7 @@ #> function Export-InforcerTenantDocumentation { [CmdletBinding()] -[OutputType([System.IO.FileInfo])] +[OutputType([hashtable], [System.IO.FileInfo])] param( [Parameter(Mandatory = $false)] [ValidateSet('Html', 'Markdown', 'Excel')] @@ -86,7 +96,10 @@ param( [object]$TenantId, [Parameter(Mandatory = $false)] - [string]$OutputPath = '.', + [string]$OutputPath, + + [Parameter(Mandatory = $false)] + [switch]$Show, [Parameter(Mandatory = $false)] [string]$SettingsCatalogPath, @@ -383,6 +396,15 @@ foreach ($product in $docModel.Products.Values) { } Write-Host " Found $policyCount policies across $($docModel.Products.Count) products" -ForegroundColor Gray +# ── No -OutputPath: hand back the DocModel and touch nothing ─────────────────── +# Writing is opt-in even here, where the verb promises a file. The old default put documents in +# whatever directory you happened to be standing in and opened a browser for them, which is +# wrong in a pipeline and wrong for a caller that only wants the model to read. +if ([string]::IsNullOrWhiteSpace($OutputPath)) { + Write-Host 'Done. Pass -OutputPath to write files, -Show to open the HTML.' -ForegroundColor Cyan + return $docModel +} + # Render each requested format and write to disk $extensionMap = @{ Html = 'html'; Markdown = 'md'; Excel = 'xlsx' } $formatIndex = 0 @@ -423,16 +445,19 @@ foreach ($fmt in $Format) { $fileInfo } -# Auto-open HTML output in the default browser (cross-platform) -$htmlFile = $Format | Where-Object { $_ -eq 'Html' } | ForEach-Object { - if ($Format.Count -eq 1 -and [System.IO.Path]::HasExtension($OutputPath)) { $OutputPath } - else { Join-Path $OutputPath "$safeName-Documentation.html" } -} -if ($htmlFile -and (Test-Path -LiteralPath $htmlFile)) { - $fullHtmlPath = (Resolve-Path -LiteralPath $htmlFile).Path - if ($IsMacOS) { Start-Process 'open' -ArgumentList $fullHtmlPath } - elseif ($IsWindows) { Start-Process $fullHtmlPath } - elseif ($IsLinux) { Start-Process 'xdg-open' -ArgumentList $fullHtmlPath } +# Opening a browser is opt-in via -Show. Doing it on every run spawned a window even inside CI +# containers, where there is nothing to open it with. +if ($Show) { + $htmlFile = $Format | Where-Object { $_ -eq 'Html' } | ForEach-Object { + if ($Format.Count -eq 1 -and [System.IO.Path]::HasExtension($OutputPath)) { $OutputPath } + else { Join-Path $OutputPath "$safeName-Documentation.html" } + } + if ($htmlFile -and (Test-Path -LiteralPath $htmlFile)) { + $fullHtmlPath = (Resolve-Path -LiteralPath $htmlFile).Path + if ($IsMacOS) { Start-Process 'open' -ArgumentList $fullHtmlPath } + elseif ($IsWindows) { Start-Process $fullHtmlPath } + elseif ($IsLinux) { Start-Process 'xdg-open' -ArgumentList $fullHtmlPath } + } } Write-Host "Done. $($Format.Count) file(s) exported for tenant '$($docModel.TenantName)'." -ForegroundColor Cyan From ece1f4aa95ea37fefb35eee1c0c94cb9f82561f1 Mon Sep 17 00:00:00 2001 From: Roy Klooster Date: Mon, 24 Aug 2026 13:58:34 +0200 Subject: [PATCH 04/20] =?UTF-8?q?docs:=20release=200.7.0=20=E2=80=94=20cha?= =?UTF-8?q?ngelog,=20cmdlet=20reference,=20versioning=20note?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps ModuleVersion to 0.7.0. The pipeline does not compute versions (Test-Version.ps1 only asserts the manifest is ahead of the Gallery), so this is set by hand; Generate-Changelog.ps1 routes the feat!/BREAKING CHANGE commit into its own Breaking Changes heading. CHANGELOG.md gains a 0.7.0 entry leading with the breaking change and its migration steps, then the four bug fixes shipped alongside it. Also corrects the versioning rule stated at the top of CHANGELOG.md. It claimed 'only breaking changes bump MAJOR', which 0.7.0 contradicts. Reworded to say what we actually do: pre-1.0, a breaking change bumps MINOR and is called out under a Breaking Changes heading, and 1.0.0 is reserved for declaring the public surface stable. docs/CMDLET-REFERENCE.md: -OutputPath rows for both affected cmdlets now say there is no default and describe the model return, and both gain a -Show row. --- CHANGELOG.md | 27 ++++++++++++++++++++++++++- docs/CMDLET-REFERENCE.md | 6 ++++-- module/InforcerCommunity.psd1 | 2 +- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34dc4ab..d3d2527 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,32 @@ All notable changes to this project will be documented in this file. -The format follows [Conventional Commits](https://www.conventionalcommits.org/). Versioning deviates from strict SemVer: every shipped change (feat / fix / perf / non-breaking refactor) bumps MINOR; only breaking changes bump MAJOR; docs/tests/chore-only commits don't bump. There is intentionally no `[Unreleased]` section — every entry is dated at ship time. +The format follows [Conventional Commits](https://www.conventionalcommits.org/). Versioning deviates from strict SemVer: every shipped change (feat / fix / perf / non-breaking refactor) bumps MINOR; docs/tests/chore-only commits don't bump. While the module is pre-1.0 a breaking change also bumps MINOR and is called out under a **Breaking Changes** heading — 1.0.0 is reserved for the point the public surface is declared stable, after which breaking changes bump MAJOR. There is intentionally no `[Unreleased]` section — every entry is dated at ship time. + +## [0.7.0] - 2026-08-24 + +### Breaking Changes + +- **`Compare-InforcerEnvironments` and `Export-InforcerTenantDocumentation` no longer write files by default, and no longer open a browser.** Both had `$OutputPath = '.'` plus an unconditional `Start-Process` on the rendered HTML, so neither could run without dropping a file into the caller's working directory and spawning a browser window. In a CI pipeline or a container that is wrong twice over: the file is litter, and there is nothing to open it with. + - Writing now happens only when `-OutputPath` is given. There is no default. `-OutputPath`'s presence *is* the opt-in — a separate `-Export` switch would carry no information the path does not already carry. + - Opening a browser now requires the new `-Show` switch. + - **Without `-OutputPath` the cmdlets return the model instead of `System.IO.FileInfo`** — the comparison hashtable from `Compare-InforcerEnvironments`, the DocModel hashtable from `Export-InforcerTenantDocumentation`. This makes it possible to read alignment scores or tenant configuration without producing artefacts. + - **Migration:** add `-OutputPath ` to any call that relied on files appearing in the working directory, and `-Show` to any call that relied on the browser opening. Scripts consuming the `FileInfo` return value need `-OutputPath` to keep that return type. + +### Bug Fixes + +- **`Invoke-InforcerAssessment` could never reach the API.** Every call failed with `400 ValidationFailure — "Unspecified content type application/json is not allowed."` `POST /beta/tenants/{id}/assessments/{assessmentId}/runs` accepts no request body and rejects *any* `Content-Type`. Removing the header from the hashtable was not enough: `Invoke-RestMethod` supplies `application/x-www-form-urlencoded` on a bodyless POST, which is rejected the same way. Now sends `-ContentType ''`, which suppresses it entirely. +- **`Invoke-InforcerAssessment -MultiTenant` crashed instead of reporting when every tenant failed.** The run loop fell through to renderers whose `-TenantResults` is mandatory, producing `Cannot bind argument to parameter 'TenantResults' because it is an empty collection` — a parameter-binding error that said nothing about why the runs failed. A single guard after the loop now covers the JSON, HTML and CSV paths and emits `NoAssessmentResults`. +- **`Compare-InforcerEnvironments -SourceBaselineId` reported a meaningless alignment score.** Scoping the source left the destination at its full policy set, so N baseline policies were compared against the destination's entire estate and every destination-only policy counted as a deviation: a 3-policy baseline against a 756-policy tenant scored **0.2%** where Inforcer's own alignment for the same pair is 100%. The docstring advertised exactly that one-sided form as an example. The destination now inherits `-SourceBaselineId` unless `-DestinationBaselineId` overrides it (same pair now scores 100% over 3 items). When the destination genuinely is not a member of the baseline it falls back to its full policy set with a warning naming the consequence, rather than erroring; an explicitly passed `-DestinationBaselineId` that fails is still an error. +- **`Compare-InforcerEnvironments -ExcludeOS` was a no-op for every value its own documentation gave as an example.** It matched only the product name (`Entra`, `Intune`, `Defender`, …), while the OS lives in the category key built from `primaryGroup` (`Windows`, `macOS`, `iOS/iPadOS`, `Android`). `-ExcludeOS 'macOS','iOS'` removed 0 of 2229 items while reporting success. It now matches the category key as well as the product name, so `-ExcludeOS 'macOS','iOS'` removes 720 items and passing a product name still works. +- **`Export-InforcerTenantDocumentation -Tag` rendered an empty document when nothing matched.** A 0.1 KB file and exit 0 reads as "nothing in this tenant is tagged that way" when the likelier cause is a tag name that does not exist. It now emits `TagMatchedNothing` and names the tags that do exist, writing no file. + +### Documentation + +- `-OutputPath`, the new `-Show` switch, and the changed return types documented in `Get-Help` and `docs/CMDLET-REFERENCE.md` for both affected cmdlets. +- `-ExcludeOS` help now states that matching applies to both product names and platform category keys. +- `-SourceBaselineId` help documents destination inheritance and the non-member fallback; the stale example claiming a one-sided comparison against "all Fabrikam policies" corrected. +- Versioning note above clarified: pre-1.0, breaking changes bump MINOR under a **Breaking Changes** heading rather than MAJOR. ## [0.6.0] - 2026-07-06 diff --git a/docs/CMDLET-REFERENCE.md b/docs/CMDLET-REFERENCE.md index 87ffc63..02c860e 100644 --- a/docs/CMDLET-REFERENCE.md +++ b/docs/CMDLET-REFERENCE.md @@ -633,7 +633,8 @@ The HTML output features a modern admin dashboard design with a collapsible side |-----------|------|-----------|--------------| | **Format** | String | No | Output format: `Html` (default), `Markdown`, `Excel`. Excel creates an .xlsx workbook with one sheet per product (requires `ImportExcel` module). | | **TenantId** | Object | Yes | Tenant to document (numeric ID, GUID, or tenant name). | -| **OutputPath** | String | No | Directory to write the output file. Defaults to current directory. | +| **OutputPath** | String | No | Directory to write the output file. **No default** — omit it and no files are written; the DocModel is returned instead. | +| **Show** | Switch | No | Open the generated HTML in the default browser. Requires `-OutputPath` and `-Format Html`. Off by default, so the cmdlet is safe in pipelines and containers. | | **SettingsCatalogPath** | String | No | Path to a local `settings.json` file for Intune Settings Catalog name resolution. When omitted, automatically downloads and caches the latest data from the [IntuneSettingsCatalogData](https://github.com/royklo/IntuneSettingsCatalogData) GitHub repository (~65 MB, cached at `~/.inforcercommunity/data/settings.json` with a 24-hour TTL). | | **FetchGraphData** | Switch | No | When set, resolves group/role/location/application GUIDs to display names across assignments and Conditional Access policies via Microsoft Graph. Also resolves assignment filter and scope tag IDs. Requires a Graph connection (use `Connect-Inforcer -FetchGraphData` or `Connect-InforcerGraph`). | | **Baseline** | String | No | Filter to policies belonging to a specific baseline (name or ID). | @@ -722,7 +723,8 @@ Compares the Intune policy configuration of two tenants and generates an interac | **ExcludeOS** | String[] | No | Exclude platforms from comparison (e.g., `'macOS'`, `'iOS'`). Case-insensitive contains matching. | | **PolicyNameFilter** | String | No | Only include policies whose name contains this string (case-insensitive). | | **SettingsCatalogPath** | String | No | Path to local `settings.json`. Auto-discovers if omitted. | -| **OutputPath** | String | No | Directory for the HTML report. Defaults to current directory. | +| **OutputPath** | String | No | Directory for the HTML report. **No default** — omit it and no file is written; the comparison model is returned instead. | +| **Show** | Switch | No | Open the generated HTML in the default browser. Requires `-OutputPath`. Off by default, so the cmdlet is safe in pipelines and containers. | ### Examples diff --git a/module/InforcerCommunity.psd1 b/module/InforcerCommunity.psd1 index e42deff..756e748 100644 --- a/module/InforcerCommunity.psd1 +++ b/module/InforcerCommunity.psd1 @@ -1,6 +1,6 @@ @{ RootModule = 'InforcerCommunity.psm1' - ModuleVersion = '0.6.0' + ModuleVersion = '0.7.0' GUID = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890' Author = 'Roy Klooster' Description = 'Community PowerShell module for the Inforcer API. Created by Roy Klooster. Not owned or officially maintained by Inforcer.' From 19bec673e096728633021ac56cad2e6beee4a456 Mon Sep 17 00:00:00 2001 From: Roy Klooster Date: Mon, 24 Aug 2026 14:10:56 +0200 Subject: [PATCH 05/20] refactor: delete 229 no-op property-alias calls Every call whose alias differed from the API name only by case did nothing. The "does this alias already exist" guard uses $o.PSObject.Properties[$aliasName], which is a case-INSENSITIVE lookup, so 'ClientTenantId' found the existing 'clientTenantId' and bailed. 229 of 238 calls, silent since they were written. Deleted rather than repaired, because making them work would have been worse than leaving them broken: - PowerShell member access is already case-insensitive. $t.ClientTenantId and even $t.TENANTFRIENDLYNAME resolve today with no alias present. There was nothing to fix. - An alias IS serialised. The 9 real renames already emit both "id" and "BaselineId" in ConvertTo-Json, and two separate Export-Csv columns. 229 more would have doubled every key and column - the same value in two casings. - -OutputType JsonObject returns before this helper runs, on purpose, so the JSON surface is the raw API shape and never carried PascalCase in the first place. The guardian skill already documented the duplicate-field hazard for JsonObject output; it just was not followed through to the conclusion that it applies to every alias, not only that one path. The 9 genuine renames are untouched and verified still firing: BaselineId<-id, BaselineName<-name, PolicyId<-id, OutputFormats<-supportedOutputFormats, Parameters<-requiredParameters, Id<-runId, OutputId<-id, OutputFormat<-format, FileSize<-sizeBytes. Per-type shape fixes are untouched too - tenant licence flattening, policy PolicyName coalescing, audit metadata flattening. No behaviour change: property access, Select-Object, ConvertTo-Json, Export-Csv and every Format.ps1xml view produce identical output. The -ObjectType ValidateSet is unchanged so all 13 call sites still work; a type needing no normalisation now simply has no branch. 462 -> 173 lines. 399 tests pass, analyzer unchanged at 2. --- CHANGELOG.md | 11 + .../Private/Add-InforcerPropertyAliases.ps1 | 353 ++---------------- 2 files changed, 43 insertions(+), 321 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3d2527..971040d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,17 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). - **`Compare-InforcerEnvironments -ExcludeOS` was a no-op for every value its own documentation gave as an example.** It matched only the product name (`Entra`, `Intune`, `Defender`, …), while the OS lives in the category key built from `primaryGroup` (`Windows`, `macOS`, `iOS/iPadOS`, `Android`). `-ExcludeOS 'macOS','iOS'` removed 0 of 2229 items while reporting success. It now matches the category key as well as the product name, so `-ExcludeOS 'macOS','iOS'` removes 720 items and passing a product name still works. - **`Export-InforcerTenantDocumentation -Tag` rendered an empty document when nothing matched.** A 0.1 KB file and exit 0 reads as "nothing in this tenant is tagged that way" when the likelier cause is a tag name that does not exist. It now emits `TagMatchedNothing` and names the tags that do exist, writing no file. +### Refactor + +- **Removed 229 no-op property-alias calls from `Add-InforcerPropertyAliases`.** Every call whose alias differed from the API name only by case did nothing: the "does this alias already exist" guard uses `$o.PSObject.Properties[$aliasName]`, which is a case-**insensitive** lookup, so `ClientTenantId` found the existing `clientTenantId` and bailed. 229 of 238 calls were affected; the 9 genuine renames (`BaselineId<-id`, `BaselineName<-name`, `PolicyId<-id`, `OutputFormats<-supportedOutputFormats`, `Parameters<-requiredParameters`, `Id<-runId`, `OutputId<-id`, `OutputFormat<-format`, `FileSize<-sizeBytes`) are unaffected and still fire. + + They were deleted rather than repaired, because making them work would have been worse than leaving them broken: + - PowerShell member access is already case-insensitive. `$tenant.ClientTenantId` — and `$tenant.TENANTFRIENDLYNAME` — resolve today with no alias present. + - An alias **is** serialised. The 9 real renames already emit both `"id"` and `"BaselineId"` in `ConvertTo-Json`, and two separate `Export-Csv` columns. Adding 229 more would have doubled every key and column, the same value in two casings. + - `-OutputType JsonObject` returns before the helper runs, deliberately, so the JSON surface is the raw API shape and never carried PascalCase to begin with. + + **No behaviour change.** Property access, `Select-Object`, `ConvertTo-Json`, `Export-Csv` and every `Format.ps1xml` view produce identical output before and after. The `-ObjectType` ValidateSet is unchanged so all 13 call sites still work; types needing no normalisation simply have no branch. + ### Documentation - `-OutputPath`, the new `-Show` switch, and the changed return types documented in `Get-Help` and `docs/CMDLET-REFERENCE.md` for both affected cmdlets. diff --git a/module/Private/Add-InforcerPropertyAliases.ps1 b/module/Private/Add-InforcerPropertyAliases.ps1 index a3b01bf..68963e6 100644 --- a/module/Private/Add-InforcerPropertyAliases.ps1 +++ b/module/Private/Add-InforcerPropertyAliases.ps1 @@ -1,15 +1,33 @@ function Add-InforcerPropertyAliases { <# .SYNOPSIS - Adds PascalCase alias properties to API response objects (Private helper). + Normalises an API response object: real property renames plus per-type shape fixes (Private helper). .DESCRIPTION - Adds PascalCase alias properties to API response objects (Private helper). - Adds aliases only when the source property exists and the alias does not. - See -ObjectType ValidateSet for supported types. + Adds an alias only where the API's own name is ambiguous or awkward — `id` on a baseline + becomes `BaselineId`, `sizeBytes` becomes `FileSize`. Nine such renames exist. + + It deliberately does NOT add PascalCase aliases for properties that differ from the API + name only by case. There used to be 229 of those calls and every one of them was a no-op: + `$o.PSObject.Properties['ClientTenantId']` is a case-INSENSITIVE lookup, so it found the + existing `clientTenantId` and the "does the alias already exist" guard bailed every time. + + That turned out to be the right behaviour arrived at by accident, so the calls were removed + rather than repaired: + - PowerShell member access is already case-insensitive. `$tenant.ClientTenantId` and even + `$tenant.TENANTFRIENDLYNAME` resolve today with no alias present. There is nothing to fix. + - An alias IS serialised. The nine real renames already emit both `"id"` and `"BaselineId"` + in ConvertTo-Json and as two separate Export-Csv columns. Adding 229 more would have + doubled every key and column in two casings — the same value, twice. + - `-OutputType JsonObject` returns before this helper runs, on purpose, so the JSON surface + is the raw API shape and never carried PascalCase in the first place. + + Some -ObjectType values now have no branch at all. That is intentional: the type needs no + normalisation, and a missing branch is a no-op. Keep passing the type from the cmdlet so + there is somewhere obvious to put a rename if the API ever needs one. .PARAMETER InputObject - The PSObject to add aliases to (e.g. from API). + The PSObject to normalise (e.g. from API). .PARAMETER ObjectType - Type of object for alias mapping. + Type of object, selecting which normalisation to apply. #> [CmdletBinding()] param( @@ -38,15 +56,6 @@ function Add-InforcerPropertyAliases { switch ($ObjectType) { 'Tenant' { - AddAliasIfExists $obj 'ClientTenantId' 'clientTenantId' - AddAliasIfExists $obj 'MsTenantId' 'msTenantId' - AddAliasIfExists $obj 'TenantFriendlyName' 'tenantFriendlyName' - AddAliasIfExists $obj 'TenantDnsName' 'tenantDnsName' - AddAliasIfExists $obj 'SecureScore' 'secureScore' - AddAliasIfExists $obj 'IsBaseline' 'isBaseline' - AddAliasIfExists $obj 'LastBackupTimestamp' 'lastBackupTimestamp' - AddAliasIfExists $obj 'RecentChanges' 'recentChanges' - AddAliasIfExists $obj 'PolicyDiff' 'policyDiff' # Licenses: replace array with comma-separated string (e.g. sku values or item ToString()) $licensesProp = $obj.PSObject.Properties['licenses'] if ($licensesProp -and $null -ne $licensesProp.Value) { @@ -69,40 +78,12 @@ function Add-InforcerPropertyAliases { } } 'Baseline' { - AddAliasIfExists $obj 'BaselineClientTenantId' 'baselineClientTenantId' + # 'id' and 'name' are ambiguous on an object that also carries baselineTenant* fields. AddAliasIfExists $obj 'BaselineId' 'id' AddAliasIfExists $obj 'BaselineName' 'name' - AddAliasIfExists $obj 'BaselineTenantFriendlyName' 'baselineTenantFriendlyName' - AddAliasIfExists $obj 'BaselineTenantDnsName' 'baselineTenantDnsName' - AddAliasIfExists $obj 'BaselineMsTenantId' 'baselineMsTenantId' - AddAliasIfExists $obj 'AlignedThreshold' 'alignedThreshold' - AddAliasIfExists $obj 'SemiAlignedThreshold' 'semiAlignedThreshold' - $membersProp = $obj.PSObject.Properties['members'] - if ($membersProp -and $membersProp.Value -is [object[]]) { - foreach ($member in $membersProp.Value) { - if ($member -is [PSObject]) { - AddAliasIfExists $member 'ClientTenantId' 'clientTenantId' - AddAliasIfExists $member 'MsTenantId' 'msTenantId' - AddAliasIfExists $member 'TenantFriendlyName' 'tenantFriendlyName' - AddAliasIfExists $member 'TenantDnsName' 'tenantDnsName' - AddAliasIfExists $member 'SecureScore' 'secureScore' - AddAliasIfExists $member 'IsBaseline' 'isBaseline' - AddAliasIfExists $member 'LastBackupTimestamp' 'lastBackupTimestamp' - AddAliasIfExists $member 'RecentChanges' 'recentChanges' - } - } - } } 'Policy' { AddAliasIfExists $obj 'PolicyId' 'id' - AddAliasIfExists $obj 'PolicyTypeId' 'policyTypeId' - AddAliasIfExists $obj 'FriendlyName' 'friendlyName' - AddAliasIfExists $obj 'ReadOnly' 'readOnly' - AddAliasIfExists $obj 'Product' 'product' - AddAliasIfExists $obj 'PrimaryGroup' 'primaryGroup' - AddAliasIfExists $obj 'SecondaryGroup' 'secondaryGroup' - AddAliasIfExists $obj 'Platform' 'platform' - AddAliasIfExists $obj 'PolicyCategoryId' 'policyCategoryId' # PolicyName: always set from displayName, name, or friendlyName (in that order); fallback "Policy {id}" $policyNameVal = $obj.PSObject.Properties['displayName'].Value -as [string] if ([string]::IsNullOrWhiteSpace($policyNameVal)) { $policyNameVal = $obj.PSObject.Properties['name'].Value -as [string] } @@ -123,70 +104,7 @@ function Add-InforcerPropertyAliases { $obj.PSObject.Properties.Add([System.Management.Automation.PSAliasProperty]::new('FriendlyName', 'PolicyName')) } } - 'AlignmentScore' { - AddAliasIfExists $obj 'TenantId' 'tenantId' - AddAliasIfExists $obj 'TenantFriendlyName' 'tenantFriendlyName' - AddAliasIfExists $obj 'Score' 'score' - AddAliasIfExists $obj 'BaselineGroupId' 'baselineGroupId' - AddAliasIfExists $obj 'BaselineGroupName' 'baselineGroupName' - AddAliasIfExists $obj 'LastComparisonDateTime' 'lastComparisonDateTime' - } - 'AlignmentDetail' { - # Top-level alignment properties - AddAliasIfExists $obj 'AlignmentScore' 'alignmentScore' - AddAliasIfExists $obj 'BaselineTenantId' 'baselineTenantId' - AddAliasIfExists $obj 'SubjectTenantId' 'subjectTenantId' - AddAliasIfExists $obj 'SubjectDataTimestamp' 'subjectDataTimestamp' - AddAliasIfExists $obj 'BaselineDataTimestamp' 'baselineDataTimestamp' - AddAliasIfExists $obj 'CompletedAt' 'completedAt' - # Metrics - $metricsProp = $obj.PSObject.Properties['metrics'] - if ($metricsProp -and $null -ne $metricsProp.Value -and $metricsProp.Value -is [PSObject]) { - $m = $metricsProp.Value - AddAliasIfExists $m 'TotalPolicies' 'totalPolicies' - AddAliasIfExists $m 'MatchedPolicies' 'matchedPolicies' - AddAliasIfExists $m 'MatchedWithAcceptedDeviations' 'matchedWithAcceptedDeviations' - AddAliasIfExists $m 'DeviatedPolicies' 'deviatedPolicies' - AddAliasIfExists $m 'RecommendedPoliciesFromBaseline' 'recommendedPoliciesFromBaseline' - AddAliasIfExists $m 'CustomerOnlyPolicies' 'customerOnlyPolicies' - } - # Per-policy aliases (matchedPolicies and deviatedUnaccepted arrays) - $alignPropCached = $obj.PSObject.Properties['alignment'] - if ($alignPropCached -and $null -ne $alignPropCached.Value) { - $alignVal = $alignPropCached.Value - } - foreach ($arrayName in @('matchedPolicies', 'matchedWithAcceptedDeviations', 'deviatedUnaccepted', 'missingFromSubjectUnaccepted', 'additionalInSubjectUnaccepted')) { - if ($null -eq $alignVal) { continue } - $policyArrayProp = $alignVal.PSObject.Properties[$arrayName] - if (-not $policyArrayProp -or $null -eq $policyArrayProp.Value) { continue } - foreach ($policy in @($policyArrayProp.Value)) { - if (-not ($policy -is [PSObject])) { continue } - AddAliasIfExists $policy 'PolicyName' 'policyName' - AddAliasIfExists $policy 'Product' 'product' - AddAliasIfExists $policy 'PrimaryGroup' 'primaryGroup' - AddAliasIfExists $policy 'SecondaryGroup' 'secondaryGroup' - AddAliasIfExists $policy 'Platform' 'platform' - AddAliasIfExists $policy 'PolicyTypeId' 'policyTypeId' - AddAliasIfExists $policy 'InforcerPolicyTypeName' 'inforcerPolicyTypeName' - AddAliasIfExists $policy 'PolicyCategoryId' 'policyCategoryId' - AddAliasIfExists $policy 'IsDeviation' 'isDeviation' - AddAliasIfExists $policy 'IsMissingFromSubject' 'isMissingFromSubject' - AddAliasIfExists $policy 'IsAdditionalInSubject' 'isAdditionalInSubject' - AddAliasIfExists $policy 'ReadOnly' 'readOnly' - } - } - } 'AuditEvent' { - AddAliasIfExists $obj 'Id' 'id' - AddAliasIfExists $obj 'CorrelationId' 'correlationId' - AddAliasIfExists $obj 'ClientId' 'clientId' - AddAliasIfExists $obj 'RelType' 'relType' - AddAliasIfExists $obj 'RelId' 'relId' - AddAliasIfExists $obj 'EventType' 'eventType' - AddAliasIfExists $obj 'Message' 'message' - AddAliasIfExists $obj 'Code' 'code' - AddAliasIfExists $obj 'User' 'user' - AddAliasIfExists $obj 'Timestamp' 'timestamp' # Flatten metadata onto the event so it works directly in the cmdlet output (no need to pipe .metadata) $meta = $obj.PSObject.Properties['metadata'].Value if ($null -ne $meta -and $meta -is [PSObject]) { @@ -226,235 +144,28 @@ function Add-InforcerPropertyAliases { # Metadata contains event-type-specific data (e.g. alertRuleCreate has createAlertRuleConfigCommand) # accessible via $event.metadata or Select-Object *. } - 'UserSummary' { - AddAliasIfExists $obj 'Id' 'id' - AddAliasIfExists $obj 'DisplayName' 'displayName' - AddAliasIfExists $obj 'UserPrincipalName' 'userPrincipalName' - AddAliasIfExists $obj 'UserType' 'userType' - AddAliasIfExists $obj 'JobTitle' 'jobTitle' - AddAliasIfExists $obj 'Department' 'department' - AddAliasIfExists $obj 'Groups' 'groups' - AddAliasIfExists $obj 'Roles' 'roles' - AddAliasIfExists $obj 'AssignedLicenses' 'assignedLicenses' - AddAliasIfExists $obj 'IsGlobalAdmin' 'isGlobalAdmin' - AddAliasIfExists $obj 'IsAccountEnabled' 'isAccountEnabled' - AddAliasIfExists $obj 'IsMfaRegistered' 'isMfaRegistered' - AddAliasIfExists $obj 'IsMfaCapable' 'isMfaCapable' - } - 'User' { - # Shared with UserSummary - AddAliasIfExists $obj 'Id' 'id' - AddAliasIfExists $obj 'DisplayName' 'displayName' - AddAliasIfExists $obj 'UserPrincipalName' 'userPrincipalName' - AddAliasIfExists $obj 'UserType' 'userType' - AddAliasIfExists $obj 'JobTitle' 'jobTitle' - AddAliasIfExists $obj 'Department' 'department' - AddAliasIfExists $obj 'AssignedLicenses' 'assignedLicenses' - AddAliasIfExists $obj 'IsGlobalAdmin' 'isGlobalAdmin' - AddAliasIfExists $obj 'IsMfaRegistered' 'isMfaRegistered' - AddAliasIfExists $obj 'IsMfaCapable' 'isMfaCapable' - - # Detail-only properties - AddAliasIfExists $obj 'GivenName' 'givenName' - AddAliasIfExists $obj 'Surname' 'surname' - AddAliasIfExists $obj 'Mail' 'mail' - AddAliasIfExists $obj 'MobilePhone' 'mobilePhone' - AddAliasIfExists $obj 'BusinessPhones' 'businessPhones' - AddAliasIfExists $obj 'OfficeLocation' 'officeLocation' - AddAliasIfExists $obj 'StreetAddress' 'streetAddress' - AddAliasIfExists $obj 'City' 'city' - AddAliasIfExists $obj 'State' 'state' - AddAliasIfExists $obj 'PostalCode' 'postalCode' - AddAliasIfExists $obj 'Country' 'country' - AddAliasIfExists $obj 'PreferredLanguage' 'preferredLanguage' - AddAliasIfExists $obj 'AccountEnabled' 'accountEnabled' - AddAliasIfExists $obj 'UsageLocation' 'usageLocation' - AddAliasIfExists $obj 'CreatedDateTime' 'createdDateTime' - AddAliasIfExists $obj 'LastPasswordChangeDateTime' 'lastPasswordChangeDateTime' - AddAliasIfExists $obj 'LastSignInDateTime' 'lastSignInDateTime' - AddAliasIfExists $obj 'CompanyName' 'companyName' - AddAliasIfExists $obj 'EmployeeId' 'employeeId' - AddAliasIfExists $obj 'EmployeeType' 'employeeType' - AddAliasIfExists $obj 'EmployeeHireDate' 'employeeHireDate' - AddAliasIfExists $obj 'MailNickname' 'mailNickname' - AddAliasIfExists $obj 'PreferredDataLocation' 'preferredDataLocation' - AddAliasIfExists $obj 'OnPremisesSyncEnabled' 'onPremisesSyncEnabled' - AddAliasIfExists $obj 'OtherMails' 'otherMails' - AddAliasIfExists $obj 'ProxyAddresses' 'proxyAddresses' - AddAliasIfExists $obj 'CreationType' 'creationType' - AddAliasIfExists $obj 'PasswordPolicies' 'passwordPolicies' - AddAliasIfExists $obj 'SignInSessionsValidFromDateTime' 'signInSessionsValidFromDateTime' - AddAliasIfExists $obj 'ImAddresses' 'imAddresses' - AddAliasIfExists $obj 'LegalAgeGroupClassification' 'legalAgeGroupClassification' - AddAliasIfExists $obj 'OnPremisesLastSyncDateTime' 'onPremisesLastSyncDateTime' - AddAliasIfExists $obj 'OnPremisesDistinguishedName' 'onPremisesDistinguishedName' - AddAliasIfExists $obj 'OnPremisesDomainName' 'onPremisesDomainName' - AddAliasIfExists $obj 'OnPremisesImmutableId' 'onPremisesImmutableId' - AddAliasIfExists $obj 'OnPremisesSecurityIdentifier' 'onPremisesSecurityIdentifier' - AddAliasIfExists $obj 'OnPremisesSamAccountName' 'onPremisesSamAccountName' - AddAliasIfExists $obj 'OnPremisesUserPrincipalName' 'onPremisesUserPrincipalName' - AddAliasIfExists $obj 'Manager' 'manager' - AddAliasIfExists $obj 'Groups' 'groups' - AddAliasIfExists $obj 'Devices' 'devices' - AddAliasIfExists $obj 'Roles' 'roles' - AddAliasIfExists $obj 'AppRoleAssignments' 'appRoleAssignments' - AddAliasIfExists $obj 'IsCloudOnly' 'isCloudOnly' - AddAliasIfExists $obj 'IsHybrid' 'isHybrid' - AddAliasIfExists $obj 'IsAllDevicesCompliant' 'isAllDevicesCompliant' - AddAliasIfExists $obj 'RiskState' 'riskState' - AddAliasIfExists $obj 'RiskDetail' 'riskDetail' - AddAliasIfExists $obj 'RiskLevel' 'riskLevel' - } - 'GroupSummary' { - AddAliasIfExists $obj 'Id' 'id' - AddAliasIfExists $obj 'DisplayName' 'displayName' - AddAliasIfExists $obj 'Description' 'description' - AddAliasIfExists $obj 'Mail' 'mail' - AddAliasIfExists $obj 'Visibility' 'visibility' - AddAliasIfExists $obj 'GroupTypes' 'groupTypes' - } - 'Group' { - AddAliasIfExists $obj 'Id' 'id' - AddAliasIfExists $obj 'DisplayName' 'displayName' - AddAliasIfExists $obj 'Description' 'description' - AddAliasIfExists $obj 'Mail' 'mail' - AddAliasIfExists $obj 'MailNickname' 'mailNickname' - AddAliasIfExists $obj 'Visibility' 'visibility' - AddAliasIfExists $obj 'MembershipRule' 'membershipRule' - AddAliasIfExists $obj 'GroupTypes' 'groupTypes' - AddAliasIfExists $obj 'CreatedDateTime' 'createdDateTime' - AddAliasIfExists $obj 'MailEnabled' 'mailEnabled' - AddAliasIfExists $obj 'OnPremisesSyncEnabled' 'onPremisesSyncEnabled' - AddAliasIfExists $obj 'Members' 'members' - } - 'Role' { - AddAliasIfExists $obj 'Id' 'id' - AddAliasIfExists $obj 'TemplateId' 'templateId' - AddAliasIfExists $obj 'DisplayName' 'displayName' - AddAliasIfExists $obj 'Description' 'description' - AddAliasIfExists $obj 'IsBuiltIn' 'isBuiltIn' - AddAliasIfExists $obj 'IsEnabled' 'isEnabled' - AddAliasIfExists $obj 'IsPrivileged' 'isPrivileged' - } - 'Assessment' { - AddAliasIfExists $obj 'Id' 'id' - AddAliasIfExists $obj 'Name' 'name' - AddAliasIfExists $obj 'Description' 'description' - AddAliasIfExists $obj 'AssessmentType' 'assessmentType' - AddAliasIfExists $obj 'LastUpdated' 'lastUpdated' - AddAliasIfExists $obj 'Created' 'created' - # Keep raw 'tags' shape intact (array OR comma-separated string from the API); - # downstream filters (e.g. Get-InforcerReportType -Tag) rely on the raw shape, - # and the Format.ps1xml view joins for display. - AddAliasIfExists $obj 'Tags' 'tags' - } 'ReportType' { - AddAliasIfExists $obj 'Key' 'key' - AddAliasIfExists $obj 'Name' 'name' - AddAliasIfExists $obj 'Description' 'description' - AddAliasIfExists $obj 'Collatable' 'collatable' # API uses 'supportedOutputFormats' (confirmed against api-uk.inforcer.com beta). - AddAliasIfExists $obj 'SupportedOutputFormats' 'supportedOutputFormats' AddAliasIfExists $obj 'OutputFormats' 'supportedOutputFormats' - AddAliasIfExists $obj 'RequiredParameters' 'requiredParameters' AddAliasIfExists $obj 'Parameters' 'requiredParameters' - # Keep raw 'tags' shape intact (array OR comma-separated string from the API); - # downstream filters (e.g. Get-InforcerReportType -Tag) rely on the raw shape, + # Note: raw 'tags' shape is left intact (array OR comma-separated string from the + # API); downstream filters (e.g. Get-InforcerReportType -Tag) rely on the raw shape, # and the Format.ps1xml view joins for display. - AddAliasIfExists $obj 'Tags' 'tags' } 'ReportRun' { - # API uses 'runId' (confirmed). Each run can batch multiple report types - # and output formats — hence the plurals. Field set verified against api-uk.inforcer.com. - AddAliasIfExists $obj 'RunId' 'runId' + # API uses 'runId'. 'Id' is what every other object type calls its identifier, so + # the alias keeps `$run.Id` working across types. AddAliasIfExists $obj 'Id' 'runId' - AddAliasIfExists $obj 'Status' 'status' - AddAliasIfExists $obj 'ReportTypes' 'reportTypes' - AddAliasIfExists $obj 'OutputFormats' 'outputFormats' - AddAliasIfExists $obj 'TriggeredByType' 'triggeredByType' - AddAliasIfExists $obj 'CreatedAt' 'createdAt' - AddAliasIfExists $obj 'StartedAt' 'startedAt' - AddAliasIfExists $obj 'CompletedAt' 'completedAt' - AddAliasIfExists $obj 'OutputCount' 'outputCount' } 'ReportOutput' { # Output record uses: id (output id), reportType, tenantId, format, sizeBytes AddAliasIfExists $obj 'OutputId' 'id' - AddAliasIfExists $obj 'Id' 'id' - AddAliasIfExists $obj 'RunId' 'runId' - AddAliasIfExists $obj 'TenantId' 'tenantId' - AddAliasIfExists $obj 'ReportType' 'reportType' AddAliasIfExists $obj 'OutputFormat' 'format' AddAliasIfExists $obj 'FileSize' 'sizeBytes' } - 'SecureScore' { - AddAliasIfExists $obj 'CurrentScore' 'currentScore' - AddAliasIfExists $obj 'CurrentScorePercentage' 'currentScorePercentage' - AddAliasIfExists $obj 'MaxScore' 'maxScore' - AddAliasIfExists $obj 'LicensedUserCount' 'licensedUserCount' - AddAliasIfExists $obj 'EnabledServices' 'enabledServices' - AddAliasIfExists $obj 'Scores' 'scores' - AddAliasIfExists $obj 'ControlProfiles' 'controlProfiles' - AddAliasIfExists $obj 'ControlCategoryScores' 'controlCategoryScores' - # Nested historic score points - $scoresProp = $obj.PSObject.Properties['scores'] - if ($scoresProp -and $null -ne $scoresProp.Value) { - foreach ($s in @($scoresProp.Value)) { - if ($s -is [PSObject]) { - AddAliasIfExists $s 'CreatedDateTime' 'createdDateTime' - AddAliasIfExists $s 'CurrentScore' 'currentScore' - AddAliasIfExists $s 'CurrentScorePercentage' 'currentScorePercentage' - AddAliasIfExists $s 'MaxScore' 'maxScore' - } - } - } - # Nested control profiles - $cpProp = $obj.PSObject.Properties['controlProfiles'] - if ($cpProp -and $null -ne $cpProp.Value) { - foreach ($c in @($cpProp.Value)) { - if ($c -is [PSObject]) { - AddAliasIfExists $c 'Id' 'id' - AddAliasIfExists $c 'Title' 'title' - AddAliasIfExists $c 'ControlCategory' 'controlCategory' - AddAliasIfExists $c 'Service' 'service' - AddAliasIfExists $c 'CurrentScore' 'currentScore' - AddAliasIfExists $c 'CurrentScorePercentage' 'currentScorePercentage' - AddAliasIfExists $c 'MaxScore' 'maxScore' - AddAliasIfExists $c 'MaxScorePercentage' 'maxScorePercentage' - AddAliasIfExists $c 'ScoreDifference' 'scoreDifference' - AddAliasIfExists $c 'ScoreDifferencePercentage' 'scoreDifferencePercentage' - AddAliasIfExists $c 'Remediation' 'remediation' - AddAliasIfExists $c 'RemediationImpact' 'remediationImpact' - AddAliasIfExists $c 'ActionUrl' 'actionUrl' - } - } - } - # Nested per-category scores - $ccsProp = $obj.PSObject.Properties['controlCategoryScores'] - if ($ccsProp -and $null -ne $ccsProp.Value) { - foreach ($cc in @($ccsProp.Value)) { - if ($cc -is [PSObject]) { - AddAliasIfExists $cc 'ControlCategory' 'controlCategory' - AddAliasIfExists $cc 'CurrentScore' 'currentScore' - AddAliasIfExists $cc 'CurrentScorePercentage' 'currentScorePercentage' - AddAliasIfExists $cc 'MaxScore' 'maxScore' - AddAliasIfExists $cc 'HistoricScores' 'historicScores' - # Historic score points inside each category - $hsProp = $cc.PSObject.Properties['historicScores'] - if ($hsProp -and $null -ne $hsProp.Value) { - foreach ($h in @($hsProp.Value)) { - if ($h -is [PSObject]) { - AddAliasIfExists $h 'CreatedDateTime' 'createdDateTime' - AddAliasIfExists $h 'CurrentScore' 'currentScore' - AddAliasIfExists $h 'CurrentScorePercentage' 'currentScorePercentage' - AddAliasIfExists $h 'MaxScore' 'maxScore' - } - } - } - } - } - } - } + # AlignmentScore, AlignmentDetail, UserSummary, User, GroupSummary, Group, Role, + # Assessment and SecureScore need no normalisation: every property they carry is + # already reachable case-insensitively under the API's own name. } $obj From 8ee3f0b11a6512e296584a405552092ac000a120 Mon Sep 17 00:00:00 2001 From: Roy Klooster Date: Mon, 24 Aug 2026 14:14:05 +0200 Subject: [PATCH 06/20] fix: -FetchGraphData no longer installs a module without asking Connect-InforcerGraph detected a missing Microsoft.Graph.Authentication and then ran Install-Module -Scope CurrentUser -Force -AllowClobber on it. -Force suppresses the untrusted-repository prompt, -AllowClobber permits overwriting commands owned by other modules, and neither was consented to. A read-only reporting module has no business mutating the machine's module state, and on a locked-down or offline host the install failed with a PowerShellGet error instead of saying what was missing. It now detects and reports, naming the install command - the same pattern Export-InforcerDocExcel already uses for ImportExcel. Reports via Write-Warning rather than Write-Error, deliberately. Every caller (Export-InforcerTenantDocumentation, Connect-Inforcer, Resolve-InforcerGraphEnrichment) already tests 'if (-not $graphCtx)' and falls back to raw ObjectIDs with a warning, which is the documented -FetchGraphData behaviour. Write-Error would have turned that graceful degradation into a hard failure for anyone running with $ErrorActionPreference = 'Stop'. ImportExcel uses Write-Error correctly because Excel export has no fallback; this does. Returns $null and clears $script:InforcerGraphConnected, matching the existing connection-failure path rather than inventing a second contract. Verified by shadowing the availability check: returns $null, emits the warning, raises 0 errors, and installs nothing. 399 tests pass. --- CHANGELOG.md | 2 ++ module/Private/Connect-InforcerGraph.ps1 | 20 +++++++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 971040d..9ef7e88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). - **`Invoke-InforcerAssessment -MultiTenant` crashed instead of reporting when every tenant failed.** The run loop fell through to renderers whose `-TenantResults` is mandatory, producing `Cannot bind argument to parameter 'TenantResults' because it is an empty collection` — a parameter-binding error that said nothing about why the runs failed. A single guard after the loop now covers the JSON, HTML and CSV paths and emits `NoAssessmentResults`. - **`Compare-InforcerEnvironments -SourceBaselineId` reported a meaningless alignment score.** Scoping the source left the destination at its full policy set, so N baseline policies were compared against the destination's entire estate and every destination-only policy counted as a deviation: a 3-policy baseline against a 756-policy tenant scored **0.2%** where Inforcer's own alignment for the same pair is 100%. The docstring advertised exactly that one-sided form as an example. The destination now inherits `-SourceBaselineId` unless `-DestinationBaselineId` overrides it (same pair now scores 100% over 3 items). When the destination genuinely is not a member of the baseline it falls back to its full policy set with a warning naming the consequence, rather than erroring; an explicitly passed `-DestinationBaselineId` that fails is still an error. - **`Compare-InforcerEnvironments -ExcludeOS` was a no-op for every value its own documentation gave as an example.** It matched only the product name (`Entra`, `Intune`, `Defender`, …), while the OS lives in the category key built from `primaryGroup` (`Windows`, `macOS`, `iOS/iPadOS`, `Android`). `-ExcludeOS 'macOS','iOS'` removed 0 of 2229 items while reporting success. It now matches the category key as well as the product name, so `-ExcludeOS 'macOS','iOS'` removes 720 items and passing a product name still works. +- **`-FetchGraphData` silently installed a module onto the machine.** `Connect-InforcerGraph` ran `Install-Module Microsoft.Graph.Authentication -Scope CurrentUser -Force -AllowClobber` with no consent when the module was absent. `-Force` suppresses the untrusted-repository prompt and `-AllowClobber` permits overwriting commands owned by other modules — neither is appropriate for a read-only reporting module, and on a locked-down or offline host it failed with a PowerShellGet error rather than saying what was missing. It now detects and reports, naming the install command, matching how the `ImportExcel` dependency is already handled. A **warning**, not an error: every caller already falls back to raw ObjectIDs on a null Graph context, and `Write-Error` would have terminated that graceful path under `$ErrorActionPreference = 'Stop'`. + - **`Export-InforcerTenantDocumentation -Tag` rendered an empty document when nothing matched.** A 0.1 KB file and exit 0 reads as "nothing in this tenant is tagged that way" when the likelier cause is a tag name that does not exist. It now emits `TagMatchedNothing` and names the tags that do exist, writing no file. ### Refactor diff --git a/module/Private/Connect-InforcerGraph.ps1 b/module/Private/Connect-InforcerGraph.ps1 index b81c1da..a09b7a3 100644 --- a/module/Private/Connect-InforcerGraph.ps1 +++ b/module/Private/Connect-InforcerGraph.ps1 @@ -21,11 +21,21 @@ function Connect-InforcerGraph { [string]$TenantId ) - # Auto-install Microsoft.Graph.Authentication if missing - $graphModule = Get-Module -ListAvailable -Name 'Microsoft.Graph.Authentication' - if (-not $graphModule) { - Write-Host ' Installing Microsoft.Graph.Authentication module...' -ForegroundColor Yellow - Install-Module -Name 'Microsoft.Graph.Authentication' -Scope CurrentUser -Force -AllowClobber + # Ask, don't install. This used to run Install-Module -Force -AllowClobber with no consent: + # -Force suppresses the untrusted-repository prompt and -AllowClobber lets it overwrite + # commands belonging to other modules. A read-only reporting module has no business mutating + # the machine's module state, and on a locked-down or offline host it failed with a + # PowerShellGet error instead of saying what was missing. Matches how the ImportExcel + # dependency is already handled in Export-InforcerDocExcel. + if (-not (Get-Module -ListAvailable -Name 'Microsoft.Graph.Authentication')) { + # A warning, not an error: every caller already handles a null context by falling back to + # raw ObjectIDs, and Write-Error here would terminate that graceful path for anyone running + # with $ErrorActionPreference = 'Stop'. The ImportExcel dependency uses Write-Error because + # Excel export has no fallback - this does. + Write-Warning ('Microsoft.Graph.Authentication is not installed, so Graph enrichment is unavailable. ' + + 'Install it with: Install-Module Microsoft.Graph.Authentication -Scope CurrentUser') + $script:InforcerGraphConnected = $false + return $null } if (-not (Get-Module -Name 'Microsoft.Graph.Authentication')) { From 95e0300490395dac50cab07b87b194bcfb5bab39 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:07:06 +0200 Subject: [PATCH 07/20] chore: gitignore the API feedback file and its evidence api-feedback.md, Reports-API-Feedback.md and api-feedback-evidence/ were untracked but not ignored, so one `git add -A` would have committed them. The evidence directory holds live response bodies captured from real customer tenants, and the feedback file quotes tenant ids, policy names and baseline ids throughout. Same reasoning as the MCP repo's docs/FINDINGS.md, which was tracked for two days because a .gitignore rule was added without a `git rm --cached`. Ignoring these before they are ever staged avoids that. Verified with git check-ignore and a dry-run `git add -A`: none of the three is stageable now. --- .gitignore | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.gitignore b/.gitignore index 760b72b..e163909 100644 --- a/.gitignore +++ b/.gitignore @@ -89,3 +89,10 @@ scripts/Test-ApiFeedbackItems.ps1 # Local API key for live-* scripts (never commit) .inforcer-key.local + +# API feedback for the Inforcer API team, and the raw captures behind it. +# Untracked on purpose: the evidence carries tenant ids, policy names and live +# response bodies from real customer tenants. +api-feedback.md +Reports-API-Feedback.md +api-feedback-evidence/ From 143a1fbf2662804357f85e1c0d7ddf795b1c3ef7 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Fri, 4 Sep 2026 20:28:36 +0200 Subject: [PATCH 08/20] fix: Markdown export named the wrong baseline in its header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ConvertTo-InforcerMarkdown printed DocModel.BaselineName, which ConvertTo-InforcerDocModel fills with the first baseline attached to the tenant — unrelated to -Baseline. Exporting the Blueprint Library filtered to 'Tier 2 - Enhanced' produced a correctly filtered 238-policy document headed 'Baseline: ... Tier 0 - Initiate', and an unfiltered export named a baseline it was not scoped to at all. The header now reads FilterBaseline, the same field the HTML renderer already used, so it names the baseline actually filtered on and is omitted when there is none. -Tag is shown in the Markdown header too, matching HTML. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 ++ Tests/Renderers.Tests.ps1 | 22 +++++++++++++++++-- module/Private/ConvertTo-InforcerMarkdown.ps1 | 13 ++++++++--- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ef7e88..9d596ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). - **`Export-InforcerTenantDocumentation -Tag` rendered an empty document when nothing matched.** A 0.1 KB file and exit 0 reads as "nothing in this tenant is tagged that way" when the likelier cause is a tag name that does not exist. It now emits `TagMatchedNothing` and names the tags that do exist, writing no file. +- **`Export-InforcerTenantDocumentation -Format Markdown` named the wrong baseline in the header.** The Markdown header printed `DocModel.BaselineName`, which `ConvertTo-InforcerDocModel` fills with the *first baseline attached to the tenant* — unrelated to `-Baseline`. Exporting the Blueprint Library filtered to `Tier 2 - Enhanced` produced a correctly filtered 238-policy document headed `*Baseline: … Tier 0 - Initiate*`, and an unfiltered export named a baseline it was not scoped to at all. The header now reads `FilterBaseline`, the same field the HTML renderer already used, so it names the baseline actually filtered on and is omitted when there is none. `-Tag` is now shown in the Markdown header too, matching HTML. + ### Refactor - **Removed 229 no-op property-alias calls from `Add-InforcerPropertyAliases`.** Every call whose alias differed from the API name only by case did nothing: the "does this alias already exist" guard uses `$o.PSObject.Properties[$aliasName]`, which is a case-**insensitive** lookup, so `ClientTenantId` found the existing `clientTenantId` and bailed. 229 of 238 calls were affected; the 9 genuine renames (`BaselineId<-id`, `BaselineName<-name`, `PolicyId<-id`, `OutputFormats<-supportedOutputFormats`, `Parameters<-requiredParameters`, `Id<-runId`, `OutputId<-id`, `OutputFormat<-format`, `FileSize<-sizeBytes`) are unaffected and still fire. diff --git a/Tests/Renderers.Tests.ps1 b/Tests/Renderers.Tests.ps1 index a9fd301..91f13c1 100644 --- a/Tests/Renderers.Tests.ps1 +++ b/Tests/Renderers.Tests.ps1 @@ -230,7 +230,10 @@ Describe 'ConvertTo-InforcerMarkdown' -Tag 'Markdown' { TenantName = 'Test Tenant' TenantId = 12345 GeneratedAt = [datetime]'2026-01-15 10:30:00' - BaselineName = 'TestBaseline' + # BaselineName is the tenant's first attached baseline and must never reach the header; + # only FilterBaseline (set when -Baseline is used) may. + BaselineName = 'WrongBaseline' + FilterBaseline = 'TestBaseline' Products = [ordered]@{ 'Intune' = @{ Categories = [ordered]@{ @@ -304,8 +307,23 @@ Describe 'ConvertTo-InforcerMarkdown' -Tag 'Markdown' { $script:MarkdownOutput | Should -Match 'Generated: 2026-01-15' } - It 'contains baseline name' { + It 'contains the filtered baseline name, not the tenant''s first baseline' { $script:MarkdownOutput | Should -Match 'Baseline: TestBaseline' + $script:MarkdownOutput | Should -Not -Match 'WrongBaseline' + } + + It 'omits the baseline header when no baseline filter was applied' { + $unfiltered = @{ + TenantName = 'Test Tenant' + TenantId = 12345 + GeneratedAt = [datetime]'2026-01-15 10:30:00' + BaselineName = 'WrongBaseline' + Products = [ordered]@{} + } + $result = InModuleScope InforcerCommunity -Parameters @{ M = $unfiltered } { + ConvertTo-InforcerMarkdown -DocModel $M + } + $result | Should -Not -Match 'Baseline:' } It 'contains TOC with product anchor links' { diff --git a/module/Private/ConvertTo-InforcerMarkdown.ps1 b/module/Private/ConvertTo-InforcerMarkdown.ps1 index f3037a3..742d2f5 100644 --- a/module/Private/ConvertTo-InforcerMarkdown.ps1 +++ b/module/Private/ConvertTo-InforcerMarkdown.ps1 @@ -70,7 +70,8 @@ function ConvertTo-InforcerMarkdown { No file I/O, no API calls. Returns the Markdown string only. .PARAMETER DocModel Hashtable from ConvertTo-InforcerDocModel containing TenantName, TenantId, - GeneratedAt, BaselineName, and Products (OrderedDictionary). + GeneratedAt, Products (OrderedDictionary), and the optional FilterBaseline / + FilterTag metadata set by Export-InforcerTenantDocumentation. .OUTPUTS [string] Complete GFM Markdown document. #> @@ -90,8 +91,14 @@ function ConvertTo-InforcerMarkdown { [void]$sb.AppendLine("*Generated: $($DocModel.GeneratedAt.ToString('yyyy-MM-dd HH:mm:ss')) UTC*") [void]$sb.AppendLine() - if (-not [string]::IsNullOrWhiteSpace($DocModel.BaselineName)) { - [void]$sb.AppendLine("*Baseline: $($DocModel.BaselineName)*") + # FilterBaseline, not BaselineName: BaselineName is just the tenant's first attached + # baseline, so it named the wrong baseline (or any baseline at all) when -Baseline filtered. + if (-not [string]::IsNullOrWhiteSpace($DocModel.FilterBaseline)) { + [void]$sb.AppendLine("*Baseline: $($DocModel.FilterBaseline)*") + [void]$sb.AppendLine() + } + if (-not [string]::IsNullOrWhiteSpace($DocModel.FilterTag)) { + [void]$sb.AppendLine("*Tag: $($DocModel.FilterTag)*") [void]$sb.AppendLine() } From a8dc1c5f8c03693f947c2ec954dcfdc508cd3b55 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Fri, 4 Sep 2026 20:28:42 +0200 Subject: [PATCH 09/20] chore: gitignore Pester's testResults.xml Pester's default TestResult.OutputPath, produced by Invoke-Pester -CI. Nothing in the repo reads it: build-and-test.yml takes the result from -PassThru in memory and there is no test-result publisher in the pipeline. A stale 145 KB copy from 26 Aug had been sitting untracked in the root. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index e163909..424f373 100644 --- a/.gitignore +++ b/.gitignore @@ -96,3 +96,8 @@ scripts/Test-ApiFeedbackItems.ps1 api-feedback.md Reports-API-Feedback.md api-feedback-evidence/ + +# Pester's default TestResult output path, produced by Invoke-Pester -CI. Nothing +# reads it: build-and-test.yml takes the result from -PassThru in memory and there +# is no test-result publisher in the pipeline. +testResults.xml From 6e3374c02d13f5dca77b2762ba0fbd36f9111866 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Mon, 7 Sep 2026 11:39:49 +0200 Subject: [PATCH 10/20] fix: Connect-Inforcer reported Connected for an expired API key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The validation probe treats a 4xx carrying the Inforcer app envelope (success/errorCode/errors) as "subscription is live, this key just lacks the scope for /beta/baselines" — deliberately, so a key holding only Reports.Read can still connect. An expired key returns that same envelope, down to errorCode 'forbidden': 403 {"data":null,"errorCode":"forbidden","success":false, "message":"A valid API key is required to access this endpoint.", "errors":["API key has expired."]} So it passed validation and every subsequent cmdlet failed with a 403 the session had just promised would not happen. Measured against api-uk.dev: an expired key returns that on 7 of 7 estate-level routes. Neither the status code, the envelope shape nor errorCode separates the two cases, and the API exposes no key-introspection endpoint, so errors[] is the only signal available. The probe now reads it and refuses to connect when it names a key-lifecycle problem. It reads errors[] only, never the top-level message: the API serves that same generic sentence for plain scope denials, and matching it would lock out every narrow-scope key — the one thing this validation must never do. Connecting on a denied probe is still correct, but now says so instead of hiding it under -Verbose: a warning states that the subscription is live and the key is not expired, but its scopes are unverified, so a later 403 is a scope gap rather than a bad session. Filed with the API team as feedback item 25, asking for a distinct errorCode for key-level failures, 401-vs-403, or GET /beta/me returning {isActive, expiresAt, scopes[]} — any of which removes the string match. Also carries doc catch-up that was already pending on this branch: the membershipRule row on GroupSummary, and the -OutputPath / -Show / -ExcludeOS descriptions for the 0.7.0 breaking changes. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 9 ++++-- Tests/Consistency.Tests.ps1 | 35 ++++++++++++++++++++ docs/API-REFERENCE.md | 1 + docs/CMDLET-REFERENCE.md | 8 ++--- module/Public/Connect-Inforcer.ps1 | 51 ++++++++++++++++++++++++++++-- 5 files changed, 96 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d596ee..a6652d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). - **`Compare-InforcerEnvironments -ExcludeOS` was a no-op for every value its own documentation gave as an example.** It matched only the product name (`Entra`, `Intune`, `Defender`, …), while the OS lives in the category key built from `primaryGroup` (`Windows`, `macOS`, `iOS/iPadOS`, `Android`). `-ExcludeOS 'macOS','iOS'` removed 0 of 2229 items while reporting success. It now matches the category key as well as the product name, so `-ExcludeOS 'macOS','iOS'` removes 720 items and passing a product name still works. - **`-FetchGraphData` silently installed a module onto the machine.** `Connect-InforcerGraph` ran `Install-Module Microsoft.Graph.Authentication -Scope CurrentUser -Force -AllowClobber` with no consent when the module was absent. `-Force` suppresses the untrusted-repository prompt and `-AllowClobber` permits overwriting commands owned by other modules — neither is appropriate for a read-only reporting module, and on a locked-down or offline host it failed with a PowerShellGet error rather than saying what was missing. It now detects and reports, naming the install command, matching how the `ImportExcel` dependency is already handled. A **warning**, not an error: every caller already falls back to raw ObjectIDs on a null Graph context, and `Write-Error` would have terminated that graceful path under `$ErrorActionPreference = 'Stop'`. +- **`Connect-Inforcer` reported `Connected` for an expired API key.** The validation probe interprets a 4xx carrying the Inforcer app envelope (`success`/`errorCode`/`errors`) as "subscription is live, this key just lacks the scope for `/beta/baselines`" — deliberately, so a key holding only `Reports.Read` can still connect. An expired key returns that *same* envelope, down to `errorCode: "forbidden"` — `403` + `{"data":null,"errorCode":"forbidden","success":false,"message":"A valid API key is required to access this endpoint.","errors":["API key has expired."]}` — so it passed validation and every subsequent cmdlet failed with a 403 the session had just promised would not happen. Neither the status code, the envelope shape nor `errorCode` separates the two cases, and the API exposes no key-introspection endpoint (no `/beta/me`, no scope list) — so `errors[]` is the only signal available. The probe now reads it and refuses to connect when it names a key-lifecycle problem (expired, revoked, deactivated, disabled). It reads `errors[]` *only*, never the top-level `message`: the API serves that same generic `a valid API key is required…` sentence for plain scope denials, and matching it would lock out every narrow-scope key — the one thing this validation must never do, since a key holding only `Reports.Read` is a legitimate caller. + + Connecting on a denied probe is still correct, but it now says so instead of hiding it under `-Verbose`: a key whose probe returned 403 gets a warning that the subscription is live and the key is not expired, but its scopes are unverified, so a later 403 is a scope gap rather than a bad session. Filed with the API team as item 25 in the feedback document, with the fix that would remove the string matching: a distinct `errorCode` for key-level failures, or `401` for an invalid credential versus `403` for an unpermitted one, or a `GET /beta/me` returning `{isActive, expiresAt, scopes[]}`. + - **`Export-InforcerTenantDocumentation -Tag` rendered an empty document when nothing matched.** A 0.1 KB file and exit 0 reads as "nothing in this tenant is tagged that way" when the likelier cause is a tag name that does not exist. It now emits `TagMatchedNothing` and names the tags that do exist, writing no file. - **`Export-InforcerTenantDocumentation -Format Markdown` named the wrong baseline in the header.** The Markdown header printed `DocModel.BaselineName`, which `ConvertTo-InforcerDocModel` fills with the *first baseline attached to the tenant* — unrelated to `-Baseline`. Exporting the Blueprint Library filtered to `Tier 2 - Enhanced` produced a correctly filtered 238-policy document headed `*Baseline: … Tier 0 - Initiate*`, and an unfiltered export named a baseline it was not scoped to at all. The header now reads `FilterBaseline`, the same field the HTML renderer already used, so it names the baseline actually filtered on and is omitted when there is none. `-Tag` is now shown in the Markdown header too, matching HTML. @@ -39,8 +43,9 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). ### Documentation -- `-OutputPath`, the new `-Show` switch, and the changed return types documented in `Get-Help` and `docs/CMDLET-REFERENCE.md` for both affected cmdlets. -- `-ExcludeOS` help now states that matching applies to both product names and platform category keys. +- `-OutputPath`, the new `-Show` switch, and the changed return types documented in `Get-Help` and `docs/CMDLET-REFERENCE.md` for both affected cmdlets. The **Output** sections of both entries in `docs/CMDLET-REFERENCE.md` still promised a `FileInfo` return and an auto-opening browser — the parameter tables had been corrected but the return-type prose had not. +- `-ExcludeOS` help now states that matching applies to both product names and platform category keys, in `docs/CMDLET-REFERENCE.md` as well as `Get-Help`. +- `membershipRule` added to the **TenantGroupSummary** schema in `docs/API-REFERENCE.md`. The API now returns it on the group *list* endpoint as well as by-ID; only the by-ID schema documented it. Picked up from the OpenAPI drift snapshot in #39, which the nightly workflow updates without touching the hand-written reference. - `-SourceBaselineId` help documents destination inheritance and the non-member fallback; the stale example claiming a one-sided comparison against "all Fabrikam policies" corrected. - Versioning note above clarified: pre-1.0, breaking changes bump MINOR under a **Breaking Changes** heading rather than MAJOR. diff --git a/Tests/Consistency.Tests.ps1 b/Tests/Consistency.Tests.ps1 index ee6a29c..fee2f27 100644 --- a/Tests/Consistency.Tests.ps1 +++ b/Tests/Consistency.Tests.ps1 @@ -1861,6 +1861,41 @@ Describe 'Private helpers (via module scope)' { $result.Status | Should -Be 'Connected' } + It 'Rejects an expired key even though it uses the Inforcer 403 envelope' { + # Verbatim body from api-uk.dev with an expired key. errorCode is 'forbidden' — + # identical to a scope denial — so only errors[] distinguishes the two. + # Must NOT report Connected. + Mock -ModuleName InforcerCommunity Invoke-WebRequest { + [PSCustomObject]@{ + StatusCode = 403 + Content = '{"data":null,"errorCode":"forbidden","success":false,"message":"A valid API key is required to access this endpoint.","errors":["API key has expired."]}' + Headers = @{} + } + } + $secure = ConvertTo-SecureString 'expired-key' -AsPlainText -Force + $err = $null + $result = Connect-Inforcer -ApiKey $secure -Region uk -ErrorAction SilentlyContinue -ErrorVariable err + $result | Should -BeNullOrEmpty + @($err).Count | Should -BeGreaterThan 0 + $err[0].Exception.Message | Should -Match 'API key has expired' + } + + It 'Connects a narrow-scope key whose 403 carries the same generic key message' { + # Same top-level message as the expired key above — the API serves it for scope + # denials too — but errors[] names a scope, not the key. Must still connect, or + # the expired-key fix would lock out every key that lacks Baselines.Read. + Mock -ModuleName InforcerCommunity Invoke-WebRequest { + [PSCustomObject]@{ + StatusCode = 403 + Content = '{"data":null,"errorCode":"forbidden","success":false,"message":"A valid API key is required to access this endpoint.","errors":["Insufficient scope: Baselines.Read is required."]}' + Headers = @{} + } + } + $secure = ConvertTo-SecureString 'reports-only-key' -AsPlainText -Force + $result = Connect-Inforcer -ApiKey $secure -Region uk -ErrorAction SilentlyContinue -WarningAction SilentlyContinue + $result.Status | Should -Be 'Connected' + } + It 'Treats APIM 401 envelope as a real auth failure' { # APIM gateway rejects the subscription: {statusCode, message} with no Inforcer markers. Mock -ModuleName InforcerCommunity Invoke-WebRequest { diff --git a/docs/API-REFERENCE.md b/docs/API-REFERENCE.md index 7b8b317..08ac3d3 100644 --- a/docs/API-REFERENCE.md +++ b/docs/API-REFERENCE.md @@ -740,6 +740,7 @@ Summary of an Entra ID group (returned from list endpoint). | description | string | No | Description of the group. | | mail | string | No | Email address of the group. | | visibility | string | No | Visibility (e.g. Public, Private). | +| membershipRule | string | No | Dynamic membership rule. Only populated for groups whose `groupTypes` contains `DynamicMembership`. | | groupTypes | array\ | Yes | Types of the group (e.g. Unified, DynamicMembership). | **PSTypeName:** `InforcerCommunity.GroupSummary` diff --git a/docs/CMDLET-REFERENCE.md b/docs/CMDLET-REFERENCE.md index 02c860e..1748a04 100644 --- a/docs/CMDLET-REFERENCE.md +++ b/docs/CMDLET-REFERENCE.md @@ -10,7 +10,7 @@ This document describes each cmdlet with parameters, usage examples, and **examp ## Connect-Inforcer -Establishes a secure connection to the Inforcer REST API. The API key is stored as a SecureString. A minimal API call validates the key before returning; on failure (e.g. wrong key or endpoint), the connection is not established. +Establishes a secure connection to the Inforcer REST API. The API key is stored as a SecureString. A minimal API call validates the key before returning; on failure (e.g. wrong key or endpoint, or an expired/revoked key), the connection is not established. A key that is valid but lacks the scope for the probe endpoint still connects — no single scope is privileged for validation — but it warns that the key's scopes are unverified, so a later 403 reads as a scope gap rather than a bad session. **Required API scope(s)**: None (session management only) @@ -662,7 +662,7 @@ Export-InforcerTenantDocumentation -Format Html -TenantId 482 -Tag "Tier 1" ### Output -Returns `FileInfo` objects for the exported file(s). HTML output auto-opens in the default browser. +With `-OutputPath`: returns `FileInfo` objects for the exported file(s). Without it: no files are written and the DocModel (hashtable) is returned instead, so the tenant configuration can be read without producing artefacts. HTML output opens in the default browser only when `-Show` is passed. ### HTML features @@ -720,7 +720,7 @@ Compares the Intune policy configuration of two tenants and generates an interac | **DestinationBaselineId** | String | No | Baseline GUID or friendly name for the destination tenant. Scopes comparison to only policies in this baseline. | | **IncludingAssignments** | Switch | No | Include assignment data in the report (informational only, does not affect score). | | **FetchGraphData** | Switch | No | Connect to Microsoft Graph to resolve group names, assignment filters, scope tags, and compliance rules. Requires `Directory.Read.All` and `DeviceManagementConfiguration.Read.All` scopes. | -| **ExcludeOS** | String[] | No | Exclude platforms from comparison (e.g., `'macOS'`, `'iOS'`). Case-insensitive contains matching. | +| **ExcludeOS** | String[] | No | OS/platform or product names to exclude. Case-insensitive contains matching, applied to both the product name (`Entra`, `Intune`, `Defender`, `Exchange`, `SharePoint`) and the category key, which is where the OS actually lives (`Windows`, `macOS`, `iOS/iPadOS`, `Android`). Examples: `'macOS'`, `'iOS'`, `'Android'`, `'Windows'`. | | **PolicyNameFilter** | String | No | Only include policies whose name contains this string (case-insensitive). | | **SettingsCatalogPath** | String | No | Path to local `settings.json`. Auto-discovers if omitted. | | **OutputPath** | String | No | Directory for the HTML report. **No default** — omit it and no file is written; the comparison model is returned instead. | @@ -757,7 +757,7 @@ Compare-InforcerEnvironments -SourceTenantId 'Contoso' -SourceBaselineId 'Tier 1 ### Output -Returns a `FileInfo` object for the exported HTML report. Auto-opens in the default browser. +With `-OutputPath`: returns a `FileInfo` object for the exported HTML report. Without it: no file is written and the comparison model (hashtable) is returned instead, so alignment scores can be read without producing artefacts. The report opens in the default browser only when `-Show` is passed. ### HTML report features diff --git a/module/Public/Connect-Inforcer.ps1 b/module/Public/Connect-Inforcer.ps1 index 7f1f3b2..473fd7b 100644 --- a/module/Public/Connect-Inforcer.ps1 +++ b/module/Public/Connect-Inforcer.ps1 @@ -4,8 +4,10 @@ .DESCRIPTION Creates an authenticated session using an API key. You can specify -Region (uk, eu, us, anz) or -BaseUrl for custom endpoints. The API key is stored as a SecureString. - Before returning Connected, a minimal API call validates the key; if it fails (e.g. wrong key for the endpoint), - the connection is not established and an error is returned. + Before returning Connected, a minimal API call validates the key; if it fails (e.g. wrong key for the + endpoint, or an expired/revoked key), the connection is not established and an error is returned. + A key that is valid but lacks the scope for the probe endpoint still connects — no single scope is + privileged for validation. .PARAMETER ApiKey The Inforcer API key. Can be SecureString or String (converted to SecureString). .PARAMETER Region @@ -104,6 +106,8 @@ if (!$PSCmdlet.ShouldProcess('Inforcer session', 'Connect')) { return } # * 4xx with Inforcer-app error envelope → key valid; APIM accepted the subscription, # the Inforcer app rejected the scope. That's # proof the subscription works. +# * 4xx whose errors[] names a key-lifecycle +# problem ("API key has expired") → key itself is dead, not a scope gap. Error out. # * 401 with APIM gateway envelope → APIM rejected the subscription itself. Real # auth failure — error out. # * Other 4xx/5xx → propagate the message. @@ -119,6 +123,7 @@ $probeUri = $baseUrlValue.TrimEnd('/') + '/beta/baselines' $probeSucceeded = $false $probeStatusCode = 0 $probeApiMessage = $null +$probeErrorsText = $null # errors[] only, without the generic top-level message $probeEnvelope = 'unknown' # 'inforcer' | 'apim' | 'unknown' $probeRawError = $null @@ -156,6 +161,19 @@ if ($probeStatusCode -ge 200 -and $probeStatusCode -lt 300) { $msgProp = $json.PSObject.Properties['message'] if ($msgProp) { $probeApiMessage = $msgProp.Value -as [string] } + # errors[] carries the actionable detail ("API key has expired."); the top-level + # message is the generic auth placeholder ("A valid API key is required to access + # this endpoint.") that the API also emits for plain scope denials. Kept in its own + # variable because the key-lifecycle test below must read errors[] ONLY — see there. + $errorsProp = $json.PSObject.Properties['errors'] + if ($errorsProp) { + $probeErrorsText = Format-InforcerErrorDetail -Errors $errorsProp.Value + if (-not [string]::IsNullOrWhiteSpace($probeErrorsText)) { + if ([string]::IsNullOrWhiteSpace($probeApiMessage)) { $probeApiMessage = $probeErrorsText } + elseif ($probeApiMessage -notlike "*$probeErrorsText*") { $probeApiMessage = "$probeApiMessage — $probeErrorsText" } + } + } + # Envelope detection: Inforcer app responses carry success / errorCode / errors; # APIM gateway responses only carry statusCode + message (no Inforcer markers). $hasInforcerMarkers = $json.PSObject.Properties['success'] -or ` @@ -170,9 +188,29 @@ if ($probeStatusCode -ge 200 -and $probeStatusCode -lt 300) { } } +# An expired / revoked / disabled key comes back in the SAME Inforcer app envelope as a +# scope denial — measured against api-uk.dev with an expired key, right down to +# errorCode 'forbidden', which a scope denial also returns: +# +# 403 {"data":null,"errorCode":"forbidden","success":false, +# "message":"A valid API key is required to access this endpoint.", +# "errors":["API key has expired."]} +# +# So neither status code, envelope shape nor errorCode can tell "your key is dead" (must +# not connect) from "your key lacks THIS scope" (must connect — see the validation +# principle above). The API has no key-introspection endpoint and no way to read a key's +# scopes, so errors[] is the only signal that exists. +# +# Read errors[] ONLY, never the joined message: the top-level "A valid API key is required +# to access this endpoint." is the generic auth placeholder the API also serves for plain +# scope denials, so matching it would lock out every narrow-scope key — exactly the +# any-scope-connects rule this cmdlet exists to protect. +$keyRejected = $probeErrorsText -match 'expired|revoked|deactivated|disabled|not active' + # Decide validation outcome from status code AND envelope shape. $keyValid = switch ($true) { $probeSucceeded { $true; break } # 200 OK + $keyRejected { $false; break } # key itself is dead ($probeStatusCode -eq 403 -and $probeEnvelope -eq 'inforcer'){ $true; break } # APIM passed, scope denied ($probeStatusCode -eq 401 -and $probeEnvelope -eq 'inforcer'){ $true; break } # Same shape, different code on some routes default { $false } @@ -180,10 +218,19 @@ $keyValid = switch ($true) { if ($keyValid -and -not $probeSucceeded) { Write-Verbose "Key validated against /beta/baselines via $probeEnvelope envelope (HTTP $probeStatusCode). Subscription is active; scope for /beta/baselines is not granted, but the session is established." + # Say this out loud rather than only under -Verbose. Connecting on a denied probe is + # correct — a Reports.Read-only key is a legitimate caller — but the key's scopes are + # unverified, so 'Connected' promises less here than it does after a 200. Without this + # line the next cmdlet's 403 looks like the session lied. + Write-Warning ('Connected, but the key''s scopes could not be verified: the probe on /beta/baselines returned HTTP {0}. The API subscription is live and the key is not expired. If a cmdlet now fails with 403, the key is missing that endpoint''s scope.' -f $probeStatusCode) } if (-not $keyValid) { $msg = switch ($true) { + $keyRejected { + "Connection failed: $($probeApiMessage.TrimEnd('.')). Generate a new API key in the Inforcer portal." + break + } ($probeStatusCode -eq 401 -and $probeEnvelope -eq 'apim') { if ($probeApiMessage) { "Connection failed: $probeApiMessage" } else { 'Connection failed: the API subscription key is invalid (APIM gateway rejection). Verify the key in the Inforcer portal.' } From 152983456ff001b98f313f6543a152c1024459d8 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 11:09:58 +0200 Subject: [PATCH 11/20] feat: Get-InforcerGroup shows MembershipRule and OnPremisesSyncEnabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both were returned by GET /beta/tenants/{id}/groups/{groupId} and dropped by the Format.ps1xml view, so the property that says what a dynamic group does, and the one that says whether a group is editable in the cloud, were invisible unless you knew to ask for Select-Object *. MembershipRule is guarded by an ItemSelectionCondition and renders only when populated, so static groups are byte-identical to before rather than gaining a permanently blank row. OnPremisesSyncEnabled renders the API's three states distinctly — True, "False (cloud-only)" for null, "False (no longer syncing)" for an explicit false. A bare boolean cannot separate "never synced" from "de-synced", and the two mean different things when deciding where a group can be changed. MembershipRule is NOT obtainable from the group list. The list endpoint emits the key and leaves it null on every group, DynamicMembership ones included — verified live: -Group on the same group id returns (user.userType -eq "Member") where the list returns null. docs/API-REFERENCE.md claimed the opposite, having been written off the OpenAPI snapshot without checking a response; corrected here, along with the changelog bullet that introduced the claim. The GroupSummary view carries the same conditional row so it lights up with no code change if the API is fixed, marked with a ponytail: comment naming the gap. 6 Pester tests cover the view logic. Verified by mutation: collapsing the three-state ScriptBlock to a plain if/else fails 2, dropping the rows fails 4. 408 pass across Tests/, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 15 +++++++- Tests/Consistency.Tests.ps1 | 51 ++++++++++++++++++++++++++ docs/API-REFERENCE.md | 4 +- docs/CMDLET-REFERENCE.md | 48 +++++++++++++++++++----- module/InforcerCommunity.Format.ps1xml | 23 ++++++++++++ module/Public/Get-InforcerGroup.ps1 | 5 +++ 6 files changed, 133 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6652d7..344ef5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format follows [Conventional Commits](https://www.conventionalcommits.org/). Versioning deviates from strict SemVer: every shipped change (feat / fix / perf / non-breaking refactor) bumps MINOR; docs/tests/chore-only commits don't bump. While the module is pre-1.0 a breaking change also bumps MINOR and is called out under a **Breaking Changes** heading — 1.0.0 is reserved for the point the public surface is declared stable, after which breaking changes bump MAJOR. There is intentionally no `[Unreleased]` section — every entry is dated at ship time. -## [0.7.0] - 2026-08-24 +## [0.7.0] - 2026-09-09 ### Breaking Changes @@ -14,6 +14,12 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). - **Without `-OutputPath` the cmdlets return the model instead of `System.IO.FileInfo`** — the comparison hashtable from `Compare-InforcerEnvironments`, the DocModel hashtable from `Export-InforcerTenantDocumentation`. This makes it possible to read alignment scores or tenant configuration without producing artefacts. - **Migration:** add `-OutputPath ` to any call that relied on files appearing in the working directory, and `-Show` to any call that relied on the browser opening. Scripts consuming the `FileInfo` return value need `-OutputPath` to keep that return type. +### Features + +- **`Get-InforcerGroup -Group` now shows `MembershipRule` and `OnPremisesSyncEnabled` by default.** Both were returned by the by-ID endpoint and dropped by the `Format.ps1xml` view, so the property that says what a dynamic group actually *does*, and the one that says whether a group is even editable in the cloud, were invisible unless you knew to ask for `Select-Object *`. `MembershipRule` is guarded by an `ItemSelectionCondition` and appears only when populated, so static groups render exactly as before instead of gaining a permanently blank line. `OnPremisesSyncEnabled` is always shown and renders the API's three states distinctly — `True`, `False (cloud-only)` for `null`, `False (no longer syncing)` for an explicit `false` — because a bare blank cannot distinguish "never synced" from "de-synced", and the two mean different things when you are deciding where a group can be changed. + + **`MembershipRule` is not obtainable from the group *list*.** `GET /beta/tenants/{id}/groups` emits the `membershipRule` key but leaves it `null` on every group, `DynamicMembership` ones included — verified against a live tenant, where `-Group` on the same group ID returns `(user.userType -eq "Member")` and the list returns `null`. `docs/API-REFERENCE.md` claimed the opposite (added in this release's Documentation section, from the #39 OpenAPI drift snapshot) and has been corrected; `onPremisesSyncEnabled` is absent from the list payload entirely. The `GroupSummary` view carries the same conditional row so it lights up with no code change if the API starts filling it, but today it renders nothing — reading rules across a tenant costs one call per dynamic group. Filed as item 26 in the API feedback document. + ### Bug Fixes - **`Invoke-InforcerAssessment` could never reach the API.** Every call failed with `400 ValidationFailure — "Unspecified content type application/json is not allowed."` `POST /beta/tenants/{id}/assessments/{assessmentId}/runs` accepts no request body and rejects *any* `Content-Type`. Removing the header from the hashtable was not enough: `Invoke-RestMethod` supplies `application/x-www-form-urlencoded` on a bodyless POST, which is rejected the same way. Now sends `-ContentType ''`, which suppresses it entirely. @@ -45,10 +51,15 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). - `-OutputPath`, the new `-Show` switch, and the changed return types documented in `Get-Help` and `docs/CMDLET-REFERENCE.md` for both affected cmdlets. The **Output** sections of both entries in `docs/CMDLET-REFERENCE.md` still promised a `FileInfo` return and an auto-opening browser — the parameter tables had been corrected but the return-type prose had not. - `-ExcludeOS` help now states that matching applies to both product names and platform category keys, in `docs/CMDLET-REFERENCE.md` as well as `Get-Help`. -- `membershipRule` added to the **TenantGroupSummary** schema in `docs/API-REFERENCE.md`. The API now returns it on the group *list* endpoint as well as by-ID; only the by-ID schema documented it. Picked up from the OpenAPI drift snapshot in #39, which the nightly workflow updates without touching the hand-written reference. +- `membershipRule` added to the **TenantGroupSummary** schema in `docs/API-REFERENCE.md`, picked up from the OpenAPI drift snapshot in #39, which the nightly workflow updates without touching the hand-written reference. The entry originally read "the API now returns it on the group *list* endpoint as well as by-ID" — the schema says so, the runtime does not: the key is emitted and always `null`. Corrected to say exactly that, with the by-ID endpoint named as the only source. A schema gaining a property is not evidence the property is populated; this one was documented off the snapshot without a live check. +- `onPremisesSyncEnabled` in the **TenantGroup** schema now documents all three states (`true` / `false` / `null`) rather than "whether synced from on-premises AD", which read as a plain boolean and gave `null` no meaning. - `-SourceBaselineId` help documents destination inheritance and the non-member fallback; the stale example claiming a one-sided comparison against "all Fabrikam policies" corrected. - Versioning note above clarified: pre-1.0, breaking changes bump MINOR under a **Breaking Changes** heading rather than MAJOR. +### Tests + +- 6 new Pester tests covering the group `Format.ps1xml` views — the only conditional display logic in the module. Three assert the `MembershipRule` row appears when a rule exists and is absent otherwise (both views); three assert `OnPremisesSyncEnabled` renders each of the API's three states distinctly. Verified by mutation: collapsing the three-state ScriptBlock to a plain `if/else` fails 2 of them, and dropping the view rows fails 4. 408 tests pass across `Tests/`, 0 failures. + ## [0.6.0] - 2026-07-06 ### Features diff --git a/Tests/Consistency.Tests.ps1 b/Tests/Consistency.Tests.ps1 index fee2f27..1a8acef 100644 --- a/Tests/Consistency.Tests.ps1 +++ b/Tests/Consistency.Tests.ps1 @@ -2175,4 +2175,55 @@ Describe 'Private helpers (via module scope)' { } } } + + Context 'Group Format.ps1xml views' { + # The group views carry the only conditional display logic in the module: MembershipRule is + # suppressed when empty, and OnPremisesSyncEnabled renders three API states that a plain + # boolean would collapse to two. Both are easy to "simplify" into a wrong answer. + BeforeAll { + function script:Format-Group ([hashtable]$Props, [string]$TypeName) { + $g = [PSCustomObject]$Props + $g.PSObject.TypeNames.Insert(0, $TypeName) + ($g | Format-List | Out-String) + } + $script:GroupBase = @{ + id = 'f44f2f5c-3160-420b-900d-5ecbede954fc'; displayName = 'G'; description = 'd' + mail = $null; visibility = $null; groupTypes = @(); membershipRule = $null + mailEnabled = $false; createdDateTime = '2026-01-01'; onPremisesSyncEnabled = $null + members = @() + } + } + + It 'Detail view shows MembershipRule when the group has one' { + $p = $script:GroupBase.Clone() + $p.groupTypes = @('DynamicMembership') + $p.membershipRule = '(user.userType -eq "Member")' + $out = script:Format-Group $p 'InforcerCommunity.Group' + $out | Should -Match 'MembershipRule\s+:\s+\(user\.userType -eq "Member"\)' + } + + It 'Detail view omits the MembershipRule row entirely for a static group' { + $out = script:Format-Group $script:GroupBase.Clone() 'InforcerCommunity.Group' + $out | Should -Not -Match 'MembershipRule' + } + + It 'Summary view omits the MembershipRule row entirely for a static group' { + $out = script:Format-Group $script:GroupBase.Clone() 'InforcerCommunity.GroupSummary' + $out | Should -Not -Match 'MembershipRule' + } + + # null and $false both mean "not syncing now" but they are NOT the same fact: null is + # "never synced" (cloud-only), $false is "was synced, no longer". Collapsing them mislabels + # a de-synced group as cloud-editable. + It 'Detail view distinguishes all three OnPremisesSyncEnabled states' -ForEach @( + @{ Value = $true; Expected = 'True' } + @{ Value = $null; Expected = 'False \(cloud-only\)' } + @{ Value = $false; Expected = 'False \(no longer syncing\)' } + ) { + $p = $script:GroupBase.Clone() + $p.onPremisesSyncEnabled = $Value + $out = script:Format-Group $p 'InforcerCommunity.Group' + $out | Should -Match "OnPremisesSyncEnabled\s+:\s+$Expected" + } + } } diff --git a/docs/API-REFERENCE.md b/docs/API-REFERENCE.md index 08ac3d3..36e490a 100644 --- a/docs/API-REFERENCE.md +++ b/docs/API-REFERENCE.md @@ -740,7 +740,7 @@ Summary of an Entra ID group (returned from list endpoint). | description | string | No | Description of the group. | | mail | string | No | Email address of the group. | | visibility | string | No | Visibility (e.g. Public, Private). | -| membershipRule | string | No | Dynamic membership rule. Only populated for groups whose `groupTypes` contains `DynamicMembership`. | +| membershipRule | string | No | Dynamic membership rule. **Always `null` on this endpoint** — the key is emitted but never filled, including for groups whose `groupTypes` contains `DynamicMembership`. Verified against a live tenant 2026-09-09. Use the by-ID endpoint to read the rule. | | groupTypes | array\ | Yes | Types of the group (e.g. Unified, DynamicMembership). | **PSTypeName:** `InforcerCommunity.GroupSummary` @@ -761,7 +761,7 @@ Full detail of an Entra ID group (returned from by-ID endpoint). | groupTypes | array\ | Yes | Types of the group. | | createdDateTime | string (datetime) | No | When the group was created. | | mailEnabled | boolean | No | Whether mail is enabled. | -| onPremisesSyncEnabled | boolean | No | Whether synced from on-premises AD. | +| onPremisesSyncEnabled | boolean | No | Whether synced from on-premises AD. Three states: `true` = synced, `false` = was synced but no longer, `null` = never synced (cloud-only). Not returned by the list endpoint at all. | | members | array\ | No | Group members (id, displayName, type). | **PSTypeName:** `InforcerCommunity.Group` diff --git a/docs/CMDLET-REFERENCE.md b/docs/CMDLET-REFERENCE.md index 1748a04..46816f5 100644 --- a/docs/CMDLET-REFERENCE.md +++ b/docs/CMDLET-REFERENCE.md @@ -485,20 +485,50 @@ Visibility : Public GroupTypes : Unified ``` +> **`MembershipRule` is not available from the list.** `GET /beta/tenants/{id}/groups` returns the +> `membershipRule` key but leaves it `null` on every group, including `DynamicMembership` ones. Only +> the by-ID endpoint fills it, so reading a dynamic group's rule needs `-Group`: +> +> ```powershell +> Get-InforcerGroup -TenantId 139 | +> Where-Object { $_.groupTypes -contains 'DynamicMembership' } | +> ForEach-Object { Get-InforcerGroup -TenantId 139 -Group $_.id } +> ``` +> +> That is one API call per dynamic group. The list view will render `MembershipRule` without code +> changes if the API starts populating it. + ### Example output (ById) ``` -DisplayName : All Company -Id : f44f2f5c-3160-420b-900d-5ecbede954fc -Description : This is the default group for everyone in the network -Mail : allcompany@contoso.onmicrosoft.com -Visibility : Public -GroupTypes : Unified -MailEnabled : True -CreatedDateTime : 2026-02-18T21:22:23+00:00 -Members : Isaiah Langer (user), Adele Vance (user) +DisplayName : SG - Entra - DUG - All Internal Users +Id : 163ba0bf-16af-4c6c-84ae-17efe1a839ae +Description : This group contains all users in the organization. +Mail : +Visibility : +GroupTypes : DynamicMembership +MembershipRule : (user.userType -eq "Member") +MailEnabled : False +CreatedDateTime : 2026-02-18T21:22:23+00:00 +OnPremisesSyncEnabled : False (cloud-only) +Members : (none) ``` +`MembershipRule` appears only when the group has one — static groups render exactly as before, with no +blank row. + +`OnPremisesSyncEnabled` is always shown, and distinguishes the API's three states rather than printing +a bare `True`/blank: + +| API value | Rendered | Meaning | +|-----------|----------|---------| +| `true` | `True` | Synced from on-premises AD | +| `null` | `False (cloud-only)` | Never synced — created in the cloud | +| `false` | `False (no longer syncing)` | Was synced from on-premises AD, no longer is | + +Both properties are on the object in raw API form (`membershipRule`, `onPremisesSyncEnabled`) and are +unaffected by the display formatting — `-OutputType JsonObject` and `Select-Object` see the raw values. + --- ## Get-InforcerRole diff --git a/module/InforcerCommunity.Format.ps1xml b/module/InforcerCommunity.Format.ps1xml index 2548ccb..725b707 100644 --- a/module/InforcerCommunity.Format.ps1xml +++ b/module/InforcerCommunity.Format.ps1xml @@ -277,6 +277,18 @@ if ($_.groupTypes) { ($_.groupTypes) -join ', ' } else { '' } + + + + -not [string]::IsNullOrWhiteSpace($_.membershipRule) + + + MembershipRule + @@ -317,6 +329,13 @@ if ($_.groupTypes) { ($_.groupTypes) -join ', ' } else { '' } + + + -not [string]::IsNullOrWhiteSpace($_.membershipRule) + + + MembershipRule + MailEnabled @@ -325,6 +344,10 @@ CreatedDateTime + + + if ($_.onPremisesSyncEnabled) { 'True' } elseif ($null -eq $_.onPremisesSyncEnabled) { 'False (cloud-only)' } else { 'False (no longer syncing)' } + if ($_.members -and $_.members.Count -gt 0) { ($_.members | ForEach-Object { $_.displayName }) -join ', ' } else { '(none)' } diff --git a/module/Public/Get-InforcerGroup.ps1 b/module/Public/Get-InforcerGroup.ps1 index 641939e..e6419cf 100644 --- a/module/Public/Get-InforcerGroup.ps1 +++ b/module/Public/Get-InforcerGroup.ps1 @@ -13,6 +13,11 @@ function Get-InforcerGroup { Use -Search for server-side prefix filtering (fast, sent to API). Use -Filter for client-side wildcard filtering (supports contains, e.g. *comp*). + -Group additionally shows MembershipRule (only when the group has one) and + OnPremisesSyncEnabled. MembershipRule is NOT available from the list: the API returns the + field but leaves it null on every group, dynamic ones included, so reading a dynamic group's + rule requires a -Group call per group. + .PARAMETER TenantId The Inforcer tenant ID. Accepts numeric ID, GUID, or tenant name. Supports pipeline input. From cb77b1c44713f9a308c26520bd4b5d57657e8bf4 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 11:26:53 +0200 Subject: [PATCH 12/20] test: guard the 0.7.0 opt-in output contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The breaking change in this release — no default -OutputPath, no browser without -Show — had no test. Nothing else in the suite asserts on where files land, so reintroducing $OutputPath = '.' or an unconditional Start-Process would have gone green. Six AST-level tests over both cmdlets: -OutputPath must declare no default, -Show must be a switch, and every Start-Process/Invoke-Item must sit inside an if ($Show) block. Parsing the param() block keeps this offline — no API key, no mocks to drift. Verified by mutation: restoring $OutputPath = '.' on Export-InforcerTenantDocumentation fails exactly one test. Also adds -Show to $expectedParameters for both cmdlets. That assertion is a documented subset check so its absence was not failing anything, but the release's headline new parameter belongs in the contract list. 414 pass across Tests/, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) --- Tests/Consistency.Tests.ps1 | 53 +++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/Tests/Consistency.Tests.ps1 b/Tests/Consistency.Tests.ps1 index 1a8acef..7957cdc 100644 --- a/Tests/Consistency.Tests.ps1 +++ b/Tests/Consistency.Tests.ps1 @@ -54,8 +54,8 @@ Describe 'Consistency contract' { 'Get-InforcerGroup' = @('TenantId', 'Search', 'Filter', 'MaxResults', 'Group', 'OutputType') 'Get-InforcerRole' = @('TenantId', 'OutputType') 'Get-InforcerSecureScore' = @('TenantId', 'OutputType') - 'Export-InforcerTenantDocumentation' = @('Format', 'TenantId', 'OutputPath', 'SettingsCatalogPath', 'FetchGraphData', 'Baseline', 'Tag') - 'Compare-InforcerEnvironments' = @('SourceTenantId', 'DestinationTenantId', 'SourceSession', 'DestinationSession', 'SourceBaselineId', 'DestinationBaselineId', 'IncludingAssignments', 'SettingsCatalogPath', 'FetchGraphData', 'ExcludeOS', 'PolicyNameFilter', 'OutputPath') + 'Export-InforcerTenantDocumentation' = @('Format', 'TenantId', 'OutputPath', 'Show', 'SettingsCatalogPath', 'FetchGraphData', 'Baseline', 'Tag') + 'Compare-InforcerEnvironments' = @('SourceTenantId', 'DestinationTenantId', 'SourceSession', 'DestinationSession', 'SourceBaselineId', 'DestinationBaselineId', 'IncludingAssignments', 'SettingsCatalogPath', 'FetchGraphData', 'ExcludeOS', 'PolicyNameFilter', 'OutputPath', 'Show') 'Get-InforcerAssessment' = @('Format', 'OutputType') 'Invoke-InforcerAssessment' = @('TenantId', 'AssessmentId', 'OutputType') 'Get-InforcerReportType' = @('Key', 'Tag', 'OutputFormat', 'Force', 'Format', 'OutputType') @@ -2227,3 +2227,52 @@ Describe 'Private helpers (via module scope)' { } } } + +Describe 'Writing files and opening a browser stay opt-in (0.7.0 breaking change)' { + # Both cmdlets used to carry $OutputPath = '.' plus an unconditional Start-Process, so neither + # could run without dropping a file in the caller's working directory and spawning a browser. + # Reintroducing either default is invisible to every other test in this suite: nothing else + # asserts on where files land. These parse the param() block directly so no API call is needed. + BeforeDiscovery { + $script:OptInCmdlets = @( + @{ Name = 'Export-InforcerTenantDocumentation' } + @{ Name = 'Compare-InforcerEnvironments' } + ) + } + + It ' declares -OutputPath with no default value' -ForEach $script:OptInCmdlets { + $path = Join-Path $PSScriptRoot "../module/Public/$Name.ps1" + $ast = [System.Management.Automation.Language.Parser]::ParseFile($path, [ref]$null, [ref]$null) + $fn = $ast.Find({ param($n) $n -is [System.Management.Automation.Language.FunctionDefinitionAst] }, $true) + $p = $fn.Body.ParamBlock.Parameters | + Where-Object { $_.Name.VariablePath.UserPath -eq 'OutputPath' } + $p | Should -Not -BeNullOrEmpty -Because "$Name must still expose -OutputPath" + $p.DefaultValue | Should -BeNullOrEmpty -Because "a default -OutputPath writes files into the caller's working directory without being asked" + } + + It ' declares -Show as a switch defaulting to off' -ForEach $script:OptInCmdlets { + $cmd = Get-Command $Name + $cmd.Parameters['Show'].ParameterType | Should -Be ([switch]) -Because 'opening a browser must be opt-in, not a valued parameter' + } + + It ' opens a browser only inside an if ($Show) block' -ForEach $script:OptInCmdlets { + $path = Join-Path $PSScriptRoot "../module/Public/$Name.ps1" + $ast = [System.Management.Automation.Language.Parser]::ParseFile($path, [ref]$null, [ref]$null) + $launchers = $ast.FindAll({ + param($n) + $n -is [System.Management.Automation.Language.CommandAst] -and + $n.GetCommandName() -in @('Start-Process', 'Invoke-Item') + }, $true) + foreach ($l in $launchers) { + # Walk up to the enclosing if-statement and confirm $Show gates it. + $guarded = $false + $node = $l.Parent + while ($node) { + if ($node -is [System.Management.Automation.Language.IfStatementAst] -and + $node.Clauses[0].Item1.Extent.Text -match '\$Show') { $guarded = $true; break } + $node = $node.Parent + } + $guarded | Should -BeTrue -Because "$($l.GetCommandName()) at line $($l.Extent.StartLineNumber) must be gated by -Show" + } + } +} From 321507eb8974223c88ef16d53711a9b9c51f4962 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:55:53 +0200 Subject: [PATCH 13/20] feat: auto-open the HTML report again, except on CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Half of 0.7.0's breaking change is reverted. The two halves had different justifications and only one survives scrutiny. A default -OutputPath writes a file nobody asked for, so that stays opt-in. But opening a report you *did* ask for, by passing -OutputPath, is the continuation of an explicit request rather than a surprise — and the original driver for suppressing it is gone. That driver was the MCP server invoking the cmdlet and spawning a browser; the MCP reads the REST API directly and never shells out to PowerShell. Verified: no child_process/spawn anywhere in its src/ or build/, runtime deps are the MCP SDK and zod, and src/manifest.ts says it outright — "a pointer, not an integration: the repos never call each other". -Show now defaults to (Test-InforcerInteractiveHost): true at a prompt, false when CI, TF_BUILD, GITHUB_ACTIONS, GITLAB_CI, JENKINS_URL, TEAMCITY_VERSION or BUILDKITE is set. An explicit -Show or -Show:$false always beats the detection, so the CI half of the original rationale still holds and a 50-tenant loop can still opt out with one flag. The detection is deliberately conservative: it answers "is this definitely not a build agent", not "is there definitely a browser". [Environment]::UserInteractive is true for most non-Windows pwsh sessions including scripted ones, so the CI variables carry the weight. 10 new tests: one per CI variable, one for an empty value not counting as CI, and an AST check that -Show's default is the helper rather than a literal. The existing guard that Start-Process/Invoke-Item stays inside if ($Show) is unchanged and still passes. Verified live: with CI=true the export writes its file and launches nothing; with CI unset the helper returns true. 424 pass across Tests/, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 6 ++-- Tests/Consistency.Tests.ps1 | 36 +++++++++++++++++-- docs/CMDLET-REFERENCE.md | 4 +-- .../Private/Test-InforcerInteractiveHost.ps1 | 31 ++++++++++++++++ .../Public/Compare-InforcerEnvironments.ps1 | 16 ++++++--- .../Export-InforcerTenantDocumentation.ps1 | 16 ++++++--- 6 files changed, 92 insertions(+), 17 deletions(-) create mode 100644 module/Private/Test-InforcerInteractiveHost.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 344ef5c..df2975f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,11 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). ### Breaking Changes -- **`Compare-InforcerEnvironments` and `Export-InforcerTenantDocumentation` no longer write files by default, and no longer open a browser.** Both had `$OutputPath = '.'` plus an unconditional `Start-Process` on the rendered HTML, so neither could run without dropping a file into the caller's working directory and spawning a browser window. In a CI pipeline or a container that is wrong twice over: the file is litter, and there is nothing to open it with. +- **`Compare-InforcerEnvironments` and `Export-InforcerTenantDocumentation` no longer write files by default.** Both had `$OutputPath = '.'` plus an unconditional `Start-Process` on the rendered HTML, so neither could run without dropping a file into the caller's working directory and spawning a browser window. In a CI pipeline or a container that is wrong twice over: the file is litter, and there is nothing to open it with. - Writing now happens only when `-OutputPath` is given. There is no default. `-OutputPath`'s presence *is* the opt-in — a separate `-Export` switch would carry no information the path does not already carry. - - Opening a browser now requires the new `-Show` switch. + - **Opening a browser is unchanged for interactive use.** The new `-Show` switch defaults to on at a prompt and off on a build agent (`CI`, `TF_BUILD`, `GITHUB_ACTIONS`, `GITLAB_CI`, `JENKINS_URL`, `TEAMCITY_VERSION`, `BUILDKITE`), via `Test-InforcerInteractiveHost`. Passing `-Show` or `-Show:$false` always beats the detection. The two halves of this change had different justifications and only one of them survived scrutiny: a default `-OutputPath` writes a file nobody asked for, but opening a report you *did* ask for by passing `-OutputPath` is the continuation of an explicit request, not a surprise. The original driver for suppressing it — the MCP server invoking the cmdlet and spawning a browser — no longer exists: the MCP reads the REST API directly and never shells out to PowerShell (`src/manifest.ts`: "a pointer, not an integration: the repos never call each other"). - **Without `-OutputPath` the cmdlets return the model instead of `System.IO.FileInfo`** — the comparison hashtable from `Compare-InforcerEnvironments`, the DocModel hashtable from `Export-InforcerTenantDocumentation`. This makes it possible to read alignment scores or tenant configuration without producing artefacts. - - **Migration:** add `-OutputPath ` to any call that relied on files appearing in the working directory, and `-Show` to any call that relied on the browser opening. Scripts consuming the `FileInfo` return value need `-OutputPath` to keep that return type. + - **Migration:** add `-OutputPath ` to any call that relied on files appearing in the working directory. Scripts consuming the `FileInfo` return value need `-OutputPath` to keep that return type. Nothing is needed for the browser: interactive callers keep the old behaviour, and CI callers that never wanted it now get a file only. Add `-Show:$false` to an interactive script that writes many reports in a loop and does not want a window per tenant. ### Features diff --git a/Tests/Consistency.Tests.ps1 b/Tests/Consistency.Tests.ps1 index 7957cdc..3079b9a 100644 --- a/Tests/Consistency.Tests.ps1 +++ b/Tests/Consistency.Tests.ps1 @@ -2250,9 +2250,41 @@ Describe 'Writing files and opening a browser stay opt-in (0.7.0 breaking change $p.DefaultValue | Should -BeNullOrEmpty -Because "a default -OutputPath writes files into the caller's working directory without being asked" } - It ' declares -Show as a switch defaulting to off' -ForEach $script:OptInCmdlets { + It ' declares -Show as a switch' -ForEach $script:OptInCmdlets { $cmd = Get-Command $Name - $cmd.Parameters['Show'].ParameterType | Should -Be ([switch]) -Because 'opening a browser must be opt-in, not a valued parameter' + $cmd.Parameters['Show'].ParameterType | Should -Be ([switch]) + } + + # -Show auto-detects: on for a human, off on a build agent. The detection is the whole + # safety story now that auto-open is back, so it gets tested directly rather than through + # the cmdlets (which would need a live tenant and two minutes per case). + It 'Test-InforcerInteractiveHost returns false when is set' -ForEach @( + @{ Var = 'CI' }, @{ Var = 'TF_BUILD' }, @{ Var = 'GITHUB_ACTIONS' } + @{ Var = 'GITLAB_CI' }, @{ Var = 'JENKINS_URL' }, @{ Var = 'TEAMCITY_VERSION' }, @{ Var = 'BUILDKITE' } + ) { + $saved = [Environment]::GetEnvironmentVariable($Var) + try { + [Environment]::SetEnvironmentVariable($Var, 'true') + & (Get-Module InforcerCommunity) { Test-InforcerInteractiveHost } | + Should -BeFalse -Because "a build agent must never have a browser launched at it" + } finally { [Environment]::SetEnvironmentVariable($Var, $saved) } + } + + It 'Test-InforcerInteractiveHost ignores an empty CI variable' { + $saved = $env:CI + try { + $env:CI = '' + & (Get-Module InforcerCommunity) { Test-InforcerInteractiveHost } | + Should -Be ([Environment]::UserInteractive) -Because 'an empty value is not "in CI"' + } finally { $env:CI = $saved } + } + + It ' resolves -Show from the interactive check, not a hardcoded default' -ForEach $script:OptInCmdlets { + $path = Join-Path $PSScriptRoot "../module/Public/$Name.ps1" + $ast = [System.Management.Automation.Language.Parser]::ParseFile($path, [ref]$null, [ref]$null) + $fn = $ast.Find({ param($n) $n -is [System.Management.Automation.Language.FunctionDefinitionAst] }, $true) + $p = $fn.Body.ParamBlock.Parameters | Where-Object { $_.Name.VariablePath.UserPath -eq 'Show' } + $p.DefaultValue.Extent.Text | Should -Match 'Test-InforcerInteractiveHost' } It ' opens a browser only inside an if ($Show) block' -ForEach $script:OptInCmdlets { diff --git a/docs/CMDLET-REFERENCE.md b/docs/CMDLET-REFERENCE.md index 46816f5..6638012 100644 --- a/docs/CMDLET-REFERENCE.md +++ b/docs/CMDLET-REFERENCE.md @@ -664,7 +664,7 @@ The HTML output features a modern admin dashboard design with a collapsible side | **Format** | String | No | Output format: `Html` (default), `Markdown`, `Excel`. Excel creates an .xlsx workbook with one sheet per product (requires `ImportExcel` module). | | **TenantId** | Object | Yes | Tenant to document (numeric ID, GUID, or tenant name). | | **OutputPath** | String | No | Directory to write the output file. **No default** — omit it and no files are written; the DocModel is returned instead. | -| **Show** | Switch | No | Open the generated HTML in the default browser. Requires `-OutputPath` and `-Format Html`. Off by default, so the cmdlet is safe in pipelines and containers. | +| **Show** | Switch | No | Open the generated HTML in the default browser. Requires `-OutputPath` and `-Format Html`. **Defaults to on interactively, off in CI** (`CI`, `TF_BUILD`, `GITHUB_ACTIONS`, `GITLAB_CI`, `JENKINS_URL`, `TEAMCITY_VERSION`, `BUILDKITE`). `-Show` forces it, `-Show:$false` suppresses it; an explicit value always wins. | | **SettingsCatalogPath** | String | No | Path to a local `settings.json` file for Intune Settings Catalog name resolution. When omitted, automatically downloads and caches the latest data from the [IntuneSettingsCatalogData](https://github.com/royklo/IntuneSettingsCatalogData) GitHub repository (~65 MB, cached at `~/.inforcercommunity/data/settings.json` with a 24-hour TTL). | | **FetchGraphData** | Switch | No | When set, resolves group/role/location/application GUIDs to display names across assignments and Conditional Access policies via Microsoft Graph. Also resolves assignment filter and scope tag IDs. Requires a Graph connection (use `Connect-Inforcer -FetchGraphData` or `Connect-InforcerGraph`). | | **Baseline** | String | No | Filter to policies belonging to a specific baseline (name or ID). | @@ -754,7 +754,7 @@ Compares the Intune policy configuration of two tenants and generates an interac | **PolicyNameFilter** | String | No | Only include policies whose name contains this string (case-insensitive). | | **SettingsCatalogPath** | String | No | Path to local `settings.json`. Auto-discovers if omitted. | | **OutputPath** | String | No | Directory for the HTML report. **No default** — omit it and no file is written; the comparison model is returned instead. | -| **Show** | Switch | No | Open the generated HTML in the default browser. Requires `-OutputPath`. Off by default, so the cmdlet is safe in pipelines and containers. | +| **Show** | Switch | No | Open the generated HTML in the default browser. Requires `-OutputPath`. **Defaults to on interactively, off in CI** (`CI`, `TF_BUILD`, `GITHUB_ACTIONS`, `GITLAB_CI`, `JENKINS_URL`, `TEAMCITY_VERSION`, `BUILDKITE`). `-Show` forces it, `-Show:$false` suppresses it; an explicit value always wins. | ### Examples diff --git a/module/Private/Test-InforcerInteractiveHost.ps1 b/module/Private/Test-InforcerInteractiveHost.ps1 new file mode 100644 index 0000000..9bf3244 --- /dev/null +++ b/module/Private/Test-InforcerInteractiveHost.ps1 @@ -0,0 +1,31 @@ +function Test-InforcerInteractiveHost { + <# + .SYNOPSIS + True when the caller is a human at a prompt, false on a CI runner or a non-interactive host. + + .DESCRIPTION + Used as the computed default for -Show on the cmdlets that render HTML. A human who asked + for a report by passing -OutputPath almost always wants to look at it, so opening it is a + continuation of an explicit request rather than a surprise. A build agent never does, and + on a headless box Start-Process either no-ops, errors, or blocks. + + Deliberately conservative: it answers "is this definitely NOT a build agent", not "is there + definitely a browser". [Environment]::UserInteractive is true for most non-Windows pwsh + sessions including scripted ones, so the CI variables carry most of the weight. Callers who + need certainty either way pass -Show or -Show:$false explicitly, which always wins. + #> + [CmdletBinding()] + [OutputType([bool])] + param() + + # Set by GitHub Actions, GitLab CI, CircleCI, Travis, AppVeyor and most others; TF_BUILD is + # Azure Pipelines, which does not set CI. + foreach ($v in 'CI', 'TF_BUILD', 'GITHUB_ACTIONS', 'GITLAB_CI', 'JENKINS_URL', 'TEAMCITY_VERSION', 'BUILDKITE') { + if (-not [string]::IsNullOrEmpty([Environment]::GetEnvironmentVariable($v))) { + Write-Verbose "Test-InforcerInteractiveHost: '$v' is set — treating as non-interactive." + return $false + } + } + + return [Environment]::UserInteractive +} diff --git a/module/Public/Compare-InforcerEnvironments.ps1 b/module/Public/Compare-InforcerEnvironments.ps1 index 42ecd71..9cd8450 100644 --- a/module/Public/Compare-InforcerEnvironments.ps1 +++ b/module/Public/Compare-InforcerEnvironments.ps1 @@ -67,8 +67,13 @@ comparison model is returned instead, so a caller can read the numbers without touching disk. There is no default - writing is always something you asked for. .PARAMETER Show - Open the generated HTML in the default browser. Requires -OutputPath. Off by default so the - cmdlet is safe in pipelines and containers, where there is no browser to open. + Open the generated HTML in the default browser. Requires -OutputPath — there is nothing to + open when no file is written. + + Defaults to ON in an interactive session and OFF on a CI runner (CI, TF_BUILD, GITHUB_ACTIONS + and similar), so a human who asked for a report gets to look at it while a build agent does + not try to launch a browser. Pass -Show to force it, -Show:$false to suppress it; an explicit + value always wins over the detection. .OUTPUTS The comparison model (hashtable) when -OutputPath is omitted, otherwise System.IO.FileInfo for the exported HTML report. @@ -142,8 +147,9 @@ param( [Parameter(Mandatory = $false)] [string]$OutputPath, + # Defaults ON for a human at a prompt, OFF on a CI runner. -Show / -Show:$false always wins. [Parameter(Mandatory = $false)] - [switch]$Show + [switch]$Show = (Test-InforcerInteractiveHost) ) # Session guard: require an active session unless both explicit sessions are provided @@ -302,7 +308,7 @@ Write-Host " Total items: $($model.TotalItems)" -ForegroundColor Gray # browser for it - wrong in a pipeline, wrong in a container, and wrong for any caller that # just wanted the numbers. if ([string]::IsNullOrWhiteSpace($OutputPath)) { - Write-Host "Done. Pass -OutputPath to write an HTML report, -Show to open it." -ForegroundColor Cyan + Write-Host "Done. Pass -OutputPath to write an HTML report." -ForegroundColor Cyan return $model } @@ -344,7 +350,7 @@ $fileInfo = Get-Item -LiteralPath $filePath $sizeKb = [math]::Round($fileInfo.Length / 1KB, 1) Write-Host " Exported: $filePath ($sizeKb KB)" -ForegroundColor Green -# Opening a browser is opt-in via -Show. Doing it unconditionally spawned a window on every +# -Show defaults to on interactively, off in CI (Test-InforcerInteractiveHost). Doing it # run, including inside CI containers where there is nothing to open it with. if ($Show) { $fullPath = (Resolve-Path -LiteralPath $filePath).Path diff --git a/module/Public/Export-InforcerTenantDocumentation.ps1 b/module/Public/Export-InforcerTenantDocumentation.ps1 index 249729b..8e9183f 100644 --- a/module/Public/Export-InforcerTenantDocumentation.ps1 +++ b/module/Public/Export-InforcerTenantDocumentation.ps1 @@ -33,8 +33,13 @@ explicit output file path. Omit it and no files are written: the DocModel is returned instead. There is no default - writing is always something you asked for. .PARAMETER Show - Open the generated HTML in the default browser. Requires -OutputPath and -Format Html. Off by - default so the cmdlet is safe in pipelines and containers, where there is no browser to open. + Open the generated HTML in the default browser. Requires -OutputPath and -Format Html — there + is nothing to open when no file is written. + + Defaults to ON in an interactive session and OFF on a CI runner (CI, TF_BUILD, GITHUB_ACTIONS + and similar), so a human who asked for a report gets to look at it while a build agent does + not try to launch a browser. Pass -Show to force it, -Show:$false to suppress it; an explicit + value always wins over the detection. .PARAMETER SettingsCatalogPath Path to a local settings.json file for Settings Catalog resolution. When omitted, the cmdlet automatically downloads and caches the latest data from the IntuneSettingsCatalogData GitHub @@ -98,8 +103,9 @@ param( [Parameter(Mandatory = $false)] [string]$OutputPath, + # Defaults ON for a human at a prompt, OFF on a CI runner. -Show / -Show:$false always wins. [Parameter(Mandatory = $false)] - [switch]$Show, + [switch]$Show = (Test-InforcerInteractiveHost), [Parameter(Mandatory = $false)] [string]$SettingsCatalogPath, @@ -401,7 +407,7 @@ Write-Host " Found $policyCount policies across $($docModel.Products.Count) pro # whatever directory you happened to be standing in and opened a browser for them, which is # wrong in a pipeline and wrong for a caller that only wants the model to read. if ([string]::IsNullOrWhiteSpace($OutputPath)) { - Write-Host 'Done. Pass -OutputPath to write files, -Show to open the HTML.' -ForegroundColor Cyan + Write-Host 'Done. Pass -OutputPath to write files.' -ForegroundColor Cyan return $docModel } @@ -445,7 +451,7 @@ foreach ($fmt in $Format) { $fileInfo } -# Opening a browser is opt-in via -Show. Doing it on every run spawned a window even inside CI +# -Show defaults to on interactively, off in CI (Test-InforcerInteractiveHost). Doing it # containers, where there is nothing to open it with. if ($Show) { $htmlFile = $Format | Where-Object { $_ -eq 'Html' } | ForEach-Object { From 9b778d71e3e1bc06c047a294db5da81c479e372f Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 14:01:31 +0200 Subject: [PATCH 14/20] docs: bring the 0.7.0 release notes and docs in line with what ships The changelog had drifted from the branch during the last few commits, and two README examples predated the breaking change. CHANGELOG: - Auto-open promoted to its own Features bullet. It was only described inside the Breaking Changes entry, where a reader scanning for new capability would never find it. - Tests section rewritten. It claimed 6 new tests and 408 passing; the release actually adds three batches (group format views, the opt-in output contract, interactive detection) for 424 passing, and the live smoke results were not recorded at all. - Documentation section notes the -Show default and the two Output sections that had gone stale. README: - The baseline example had no -OutputPath, so under 0.7.0 it writes nothing while its comment promises a document. Added the flag. - Export and Compare rows now say -OutputPath is what writes files, and the quick-start explains the interactive/CI browser behaviour. The README was the only doc still silent on the release's breaking change. docs/CMDLET-REFERENCE.md: - Both Output sections still said the report "opens in the default browser only when -Show is passed", untrue since 321507e. Verified: 424 pass across Tests/, 0 failures. README cmdlet table is 21 rows against 21 in FunctionsToExport. .psd1 0.7.0, CHANGELOG top entry 0.7.0, main 0.6.0. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 14 +++++++++++++- README.md | 10 ++++++---- docs/CMDLET-REFERENCE.md | 4 ++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df2975f..e77b84b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). ### Features +- **HTML reports open automatically again in an interactive session — and never on a CI runner.** `Compare-InforcerEnvironments` and `Export-InforcerTenantDocumentation` gained `-Show`, which defaults to `(Test-InforcerInteractiveHost)`: true at a prompt, false when `CI`, `TF_BUILD`, `GITHUB_ACTIONS`, `GITLAB_CI`, `JENKINS_URL`, `TEAMCITY_VERSION` or `BUILDKITE` is set. `-Show` forces it and `-Show:$false` suppresses it; an explicit value always beats the detection, so a script writing one report per tenant in a loop opts out with a single flag. The detection is deliberately conservative — it answers "is this definitely not a build agent", not "is there definitely a browser", because `[Environment]::UserInteractive` is true for most non-Windows PowerShell sessions including scripted ones, leaving the CI variables to carry the weight. See **Breaking Changes** for why only half of the original opt-in change survived. + - **`Get-InforcerGroup -Group` now shows `MembershipRule` and `OnPremisesSyncEnabled` by default.** Both were returned by the by-ID endpoint and dropped by the `Format.ps1xml` view, so the property that says what a dynamic group actually *does*, and the one that says whether a group is even editable in the cloud, were invisible unless you knew to ask for `Select-Object *`. `MembershipRule` is guarded by an `ItemSelectionCondition` and appears only when populated, so static groups render exactly as before instead of gaining a permanently blank line. `OnPremisesSyncEnabled` is always shown and renders the API's three states distinctly — `True`, `False (cloud-only)` for `null`, `False (no longer syncing)` for an explicit `false` — because a bare blank cannot distinguish "never synced" from "de-synced", and the two mean different things when you are deciding where a group can be changed. **`MembershipRule` is not obtainable from the group *list*.** `GET /beta/tenants/{id}/groups` emits the `membershipRule` key but leaves it `null` on every group, `DynamicMembership` ones included — verified against a live tenant, where `-Group` on the same group ID returns `(user.userType -eq "Member")` and the list returns `null`. `docs/API-REFERENCE.md` claimed the opposite (added in this release's Documentation section, from the #39 OpenAPI drift snapshot) and has been corrected; `onPremisesSyncEnabled` is absent from the list payload entirely. The `GroupSummary` view carries the same conditional row so it lights up with no code change if the API starts filling it, but today it renders nothing — reading rules across a tenant costs one call per dynamic group. Filed as item 26 in the API feedback document. @@ -55,10 +57,20 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). - `onPremisesSyncEnabled` in the **TenantGroup** schema now documents all three states (`true` / `false` / `null`) rather than "whether synced from on-premises AD", which read as a plain boolean and gave `null` no meaning. - `-SourceBaselineId` help documents destination inheritance and the non-member fallback; the stale example claiming a one-sided comparison against "all Fabrikam policies" corrected. - Versioning note above clarified: pre-1.0, breaking changes bump MINOR under a **Breaking Changes** heading rather than MAJOR. +- `-Show`'s interactive/CI default documented in `Get-Help` and the `docs/CMDLET-REFERENCE.md` parameter tables for both cmdlets, and the **Output** sections of both entries corrected — they still said the report "opens in the default browser only when `-Show` is passed", which stopped being true when the default changed. +- `Get-InforcerGroup` example output in `docs/CMDLET-REFERENCE.md` now shows a real dynamic group with its `MembershipRule` and the `OnPremisesSyncEnabled` three-state table, plus the per-group call needed to read rules in bulk. ### Tests -- 6 new Pester tests covering the group `Format.ps1xml` views — the only conditional display logic in the module. Three assert the `MembershipRule` row appears when a rule exists and is absent otherwise (both views); three assert `OnPremisesSyncEnabled` renders each of the API's three states distinctly. Verified by mutation: collapsing the three-state ScriptBlock to a plain `if/else` fails 2 of them, and dropping the view rows fails 4. 408 tests pass across `Tests/`, 0 failures. +**424 pass across `Tests/`, 0 failures, 2 legitimate skips** (was 394 at 0.6.0). Three additions, each covering behaviour this release changed that nothing else asserted on: + +- **Group `Format.ps1xml` views (6 tests)** — the only conditional display logic in the module. Three assert the `MembershipRule` row appears when a rule exists and is absent otherwise, in both views; three assert `OnPremisesSyncEnabled` renders each of the API's three states distinctly. Verified by mutation: collapsing the three-state ScriptBlock to a plain `if/else` fails 2, and dropping the view rows fails 4. +- **The opt-in output contract (6 tests)** — AST-level checks that `-OutputPath` declares no default on either cmdlet, that `-Show` is a switch, and that every `Start-Process`/`Invoke-Item` sits inside an `if ($Show)` block. Nothing else in the suite asserted on *where files land*, so reintroducing `$OutputPath = '.'` would previously have gone green. Verified by mutation: restoring the default fails exactly one test. Parsing the param block keeps this offline — no API key, no mocks to drift. +- **Interactive detection (10 tests)** — one per CI variable, one confirming an empty value does not count as "in CI", and an AST check that `-Show`'s default is `Test-InforcerInteractiveHost` rather than a literal. + +`-Show` was also added to `$expectedParameters` for both utility cmdlets. That assertion is a documented *subset* check, so its absence was not failing anything — but the release's headline new parameter belongs in the contract list. + +**Pre-merge live smoke** (`Tests/Manual/Live-ApiSmoke.ps1`, gitignored) run against a live tenant with 21 groups, 10 of them dynamic: 12/12 pass. Covers auth including refusal of a bogus key, group list/by-ID PSTypeNames, `MembershipRule` and `OnPremisesSyncEnabled` rendering, JSON raw-camelCase-only, the surviving aliases, and the opt-in output contract end to end — no `-OutputPath` writes nothing and returns the model, with it writes a 2.7 MB HTML and returns `FileInfo`. Separately confirmed live: the assessment `-ContentType ''` fix returns 21 checks with no `400`, `-ExcludeOS 'macOS','iOS'` removes 933 of 2715 items, destination baseline inheritance scores 100% where the bug scored 0.2%, and `CI=true` writes the file while launching nothing. ## [0.6.0] - 2026-07-06 diff --git a/README.md b/README.md index 661588c..c20a79e 100644 --- a/README.md +++ b/README.md @@ -82,12 +82,14 @@ Get-InforcerTenantPolicies -TenantId 482 # Show policy changes (PolicyDiff on each tenant when available) Get-InforcerTenant | Select-Object ClientTenantId, TenantFriendlyName, PolicyDiff -# Generate tenant documentation as HTML +# Generate tenant documentation as HTML. -OutputPath is what writes the file: omit it and the +# DocModel is returned instead, so you can read the configuration without producing artefacts. +# The report opens in your browser automatically, unless you are on a CI runner or pass -Show:$false. Export-InforcerTenantDocumentation -Format Html -TenantId 482 -OutputPath ./docs # Generate documentation for a specific baseline with Graph group name resolution Connect-Inforcer -ApiKey "your-api-key" -Region uk -FetchGraphData -Export-InforcerTenantDocumentation -Format Html -TenantId 482 -Baseline "Production" -FetchGraphData +Export-InforcerTenantDocumentation -Format Html -TenantId 482 -Baseline "Production" -FetchGraphData -OutputPath ./docs # Disconnect when done Disconnect-Inforcer @@ -111,8 +113,8 @@ Disconnect-Inforcer | **Get-InforcerGroup** | Retrieves Entra ID groups from a tenant (list/search or detail by GroupId). | | **Get-InforcerRole** | Retrieves Entra ID directory role definitions from a tenant. | | **Get-InforcerSecureScore** | Retrieves the current and 90-day historic Microsoft Secure Score for a tenant, including per-category scores and actionable control profiles. | -| **Export-InforcerTenantDocumentation** | Generates comprehensive tenant documentation in HTML, Markdown, or Excel format. | -| **Compare-InforcerEnvironments** | Compares two tenants' Intune configuration and generates an interactive HTML comparison report. Supports baseline-scoped comparison via `-SourceBaselineId` / `-DestinationBaselineId` with automatic baseline owner resolution. | +| **Export-InforcerTenantDocumentation** | Generates comprehensive tenant documentation in HTML, Markdown, or Excel format. Pass `-OutputPath` to write files; without it the DocModel is returned and nothing is written. | +| **Compare-InforcerEnvironments** | Compares two tenants' Intune configuration and generates an interactive HTML comparison report. Pass `-OutputPath` to write the file; without it the comparison model is returned and nothing is written. Supports baseline-scoped comparison via `-SourceBaselineId` / `-DestinationBaselineId` with automatic baseline owner resolution. | | **Get-InforcerAssessment** | Lists available assessments (CIS, Essential Eight, etc.). | | **Invoke-InforcerAssessment** | Runs an assessment against one or more tenants. Supports HTML/CSV export. | | **Get-InforcerReportType** | Lists the report catalog from the Reports API. Cached after first call; supports `-Key`, `-Tag`, and `-OutputFormat` filters. | diff --git a/docs/CMDLET-REFERENCE.md b/docs/CMDLET-REFERENCE.md index 6638012..3e92939 100644 --- a/docs/CMDLET-REFERENCE.md +++ b/docs/CMDLET-REFERENCE.md @@ -692,7 +692,7 @@ Export-InforcerTenantDocumentation -Format Html -TenantId 482 -Tag "Tier 1" ### Output -With `-OutputPath`: returns `FileInfo` objects for the exported file(s). Without it: no files are written and the DocModel (hashtable) is returned instead, so the tenant configuration can be read without producing artefacts. HTML output opens in the default browser only when `-Show` is passed. +With `-OutputPath`: returns `FileInfo` objects for the exported file(s). Without it: no files are written and the DocModel (hashtable) is returned instead, so the tenant configuration can be read without producing artefacts. HTML output opens in the default browser automatically in an interactive session, and not on a CI runner — see `-Show`. ### HTML features @@ -787,7 +787,7 @@ Compare-InforcerEnvironments -SourceTenantId 'Contoso' -SourceBaselineId 'Tier 1 ### Output -With `-OutputPath`: returns a `FileInfo` object for the exported HTML report. Without it: no file is written and the comparison model (hashtable) is returned instead, so alignment scores can be read without producing artefacts. The report opens in the default browser only when `-Show` is passed. +With `-OutputPath`: returns a `FileInfo` object for the exported HTML report. Without it: no file is written and the comparison model (hashtable) is returned instead, so alignment scores can be read without producing artefacts. The report opens in the default browser automatically in an interactive session, and not on a CI runner — see `-Show`. ### HTML report features From e1e7d4177a1e3217314912141352aa25fe78f998 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:44:07 +0200 Subject: [PATCH 15/20] docs: drop the Refactor and Tests sections from the 0.7.0 entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Neither described anything a user can observe. The alias removal was a no-op by definition — the 229 calls it deleted never did anything, so there is no behaviour to announce — and test counts are a property of the repo, not of the release. Both are recoverable from git history and FINDINGS.md if anyone needs the reasoning. 0.7.0 keeps Breaking Changes, Features, Bug Fixes and Documentation. Earlier releases keep their own Tests sections untouched; this is not a retroactive change to shipped entries. Pure deletion — zero lines added. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e77b84b..ab1c647 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,17 +38,6 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). - **`Export-InforcerTenantDocumentation -Format Markdown` named the wrong baseline in the header.** The Markdown header printed `DocModel.BaselineName`, which `ConvertTo-InforcerDocModel` fills with the *first baseline attached to the tenant* — unrelated to `-Baseline`. Exporting the Blueprint Library filtered to `Tier 2 - Enhanced` produced a correctly filtered 238-policy document headed `*Baseline: … Tier 0 - Initiate*`, and an unfiltered export named a baseline it was not scoped to at all. The header now reads `FilterBaseline`, the same field the HTML renderer already used, so it names the baseline actually filtered on and is omitted when there is none. `-Tag` is now shown in the Markdown header too, matching HTML. -### Refactor - -- **Removed 229 no-op property-alias calls from `Add-InforcerPropertyAliases`.** Every call whose alias differed from the API name only by case did nothing: the "does this alias already exist" guard uses `$o.PSObject.Properties[$aliasName]`, which is a case-**insensitive** lookup, so `ClientTenantId` found the existing `clientTenantId` and bailed. 229 of 238 calls were affected; the 9 genuine renames (`BaselineId<-id`, `BaselineName<-name`, `PolicyId<-id`, `OutputFormats<-supportedOutputFormats`, `Parameters<-requiredParameters`, `Id<-runId`, `OutputId<-id`, `OutputFormat<-format`, `FileSize<-sizeBytes`) are unaffected and still fire. - - They were deleted rather than repaired, because making them work would have been worse than leaving them broken: - - PowerShell member access is already case-insensitive. `$tenant.ClientTenantId` — and `$tenant.TENANTFRIENDLYNAME` — resolve today with no alias present. - - An alias **is** serialised. The 9 real renames already emit both `"id"` and `"BaselineId"` in `ConvertTo-Json`, and two separate `Export-Csv` columns. Adding 229 more would have doubled every key and column, the same value in two casings. - - `-OutputType JsonObject` returns before the helper runs, deliberately, so the JSON surface is the raw API shape and never carried PascalCase to begin with. - - **No behaviour change.** Property access, `Select-Object`, `ConvertTo-Json`, `Export-Csv` and every `Format.ps1xml` view produce identical output before and after. The `-ObjectType` ValidateSet is unchanged so all 13 call sites still work; types needing no normalisation simply have no branch. - ### Documentation - `-OutputPath`, the new `-Show` switch, and the changed return types documented in `Get-Help` and `docs/CMDLET-REFERENCE.md` for both affected cmdlets. The **Output** sections of both entries in `docs/CMDLET-REFERENCE.md` still promised a `FileInfo` return and an auto-opening browser — the parameter tables had been corrected but the return-type prose had not. @@ -60,18 +49,6 @@ The format follows [Conventional Commits](https://www.conventionalcommits.org/). - `-Show`'s interactive/CI default documented in `Get-Help` and the `docs/CMDLET-REFERENCE.md` parameter tables for both cmdlets, and the **Output** sections of both entries corrected — they still said the report "opens in the default browser only when `-Show` is passed", which stopped being true when the default changed. - `Get-InforcerGroup` example output in `docs/CMDLET-REFERENCE.md` now shows a real dynamic group with its `MembershipRule` and the `OnPremisesSyncEnabled` three-state table, plus the per-group call needed to read rules in bulk. -### Tests - -**424 pass across `Tests/`, 0 failures, 2 legitimate skips** (was 394 at 0.6.0). Three additions, each covering behaviour this release changed that nothing else asserted on: - -- **Group `Format.ps1xml` views (6 tests)** — the only conditional display logic in the module. Three assert the `MembershipRule` row appears when a rule exists and is absent otherwise, in both views; three assert `OnPremisesSyncEnabled` renders each of the API's three states distinctly. Verified by mutation: collapsing the three-state ScriptBlock to a plain `if/else` fails 2, and dropping the view rows fails 4. -- **The opt-in output contract (6 tests)** — AST-level checks that `-OutputPath` declares no default on either cmdlet, that `-Show` is a switch, and that every `Start-Process`/`Invoke-Item` sits inside an `if ($Show)` block. Nothing else in the suite asserted on *where files land*, so reintroducing `$OutputPath = '.'` would previously have gone green. Verified by mutation: restoring the default fails exactly one test. Parsing the param block keeps this offline — no API key, no mocks to drift. -- **Interactive detection (10 tests)** — one per CI variable, one confirming an empty value does not count as "in CI", and an AST check that `-Show`'s default is `Test-InforcerInteractiveHost` rather than a literal. - -`-Show` was also added to `$expectedParameters` for both utility cmdlets. That assertion is a documented *subset* check, so its absence was not failing anything — but the release's headline new parameter belongs in the contract list. - -**Pre-merge live smoke** (`Tests/Manual/Live-ApiSmoke.ps1`, gitignored) run against a live tenant with 21 groups, 10 of them dynamic: 12/12 pass. Covers auth including refusal of a bogus key, group list/by-ID PSTypeNames, `MembershipRule` and `OnPremisesSyncEnabled` rendering, JSON raw-camelCase-only, the surviving aliases, and the opt-in output contract end to end — no `-OutputPath` writes nothing and returns the model, with it writes a 2.7 MB HTML and returns `FileInfo`. Separately confirmed live: the assessment `-ContentType ''` fix returns 21 checks with no `400`, `-ExcludeOS 'macOS','iOS'` removes 933 of 2715 items, destination baseline inheritance scores 100% where the bug scored 0.2%, and `CI=true` writes the file while launching nothing. - ## [0.6.0] - 2026-07-06 ### Features From 74aef8787fa4f3127e16579c1f63e5c126a991b9 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:09:25 +0200 Subject: [PATCH 16/20] test: cover the three 0.7.0 fixes that only live runs reached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These shipped verified by hand and by a live smoke, with nothing in CI to catch a regression. Each test targets the smallest seam that encodes the fix. Invoke-InforcerAssessmentRun (2 tests) — asserts the run POST passes -ContentType "" and that no Content-Type header is set anywhere in the file. Source-level rather than a Mock: the call runs in a separate runspace via AddScript, so a module-scoped Mock cannot reach it, which makes the script text the actual contract. -ExcludeOS (4 tests) — synthetic DocModels with Windows, macOS and iOS categories under one product. Asserts a category is dropped when its KEY contains the excluded OS, that several can go at once, that only the named OS goes, and that excluding a PRODUCT name still works — that last was the only behaviour the original code had, so it is the thing most likely to be broken by a careless fix. Destination baseline inheritance (4 tests) — mocks Get-InforcerDocData and Select-InforcerBaselinePolicies, then asserts the destination is scoped to the source baseline when none is given, that an explicit -DestinationBaselineId still wins, that an inherited baseline the destination is not a member of warns and falls back rather than erroring, and that an explicit one that fails is still an error. All three mutation-verified. Reintroducing the Content-Type fails 1; reverting -ExcludeOS to product-name-only fails 2; removing the inheritance branch fails 2. 434 pass across Tests/, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) --- Tests/Consistency.Tests.ps1 | 176 ++++++++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+) diff --git a/Tests/Consistency.Tests.ps1 b/Tests/Consistency.Tests.ps1 index 3079b9a..2fb1961 100644 --- a/Tests/Consistency.Tests.ps1 +++ b/Tests/Consistency.Tests.ps1 @@ -2308,3 +2308,179 @@ Describe 'Writing files and opening a browser stay opt-in (0.7.0 breaking change } } } + +Describe '0.7.0 bug fixes that only live runs covered' { + # These three shipped verified by hand and by a live smoke, with nothing in CI to catch a + # regression. Each targets the smallest seam that actually encodes the fix. + + Context 'Invoke-InforcerAssessmentRun sends no Content-Type' { + # POST /beta/tenants/{id}/assessments/{id}/runs takes no body and rejects ANY Content-Type + # with 400 ValidationFailure. Dropping the header is not enough — Invoke-RestMethod supplies + # application/x-www-form-urlencoded on a bodyless POST, which is rejected identically. + # The call runs in a separate runspace via AddScript, so a module-scoped Mock cannot reach + # it; the script text IS the contract, so that is what gets asserted. + BeforeAll { + $script:RunnerSrc = Get-Content (Join-Path $PSScriptRoot '../module/Private/Invoke-InforcerAssessmentRun.ps1') -Raw + } + + It 'passes -ContentType "" on the run POST' { + $script:RunnerSrc | Should -Match 'Invoke-RestMethod[^\r\n]*-ContentType\s*""' + } + + It 'never sets a Content-Type header on the runs endpoint' { + # A 'Content-Type' = 'application/json' entry anywhere in this file would reintroduce the 400. + $script:RunnerSrc | Should -Not -Match "(?i)['\`"]Content-Type['\`"]\s*=" + } + } + + Context '-ExcludeOS matches the category key, not just the product name' { + # The OS lives in the category key built from primaryGroup (Windows, macOS, iOS/iPadOS, + # Android) and never in the product name (Entra, Intune, Defender). Matching products alone + # made every documented example value a silent no-op that removed 0 items and reported success. + BeforeAll { + function script:New-TestPolicy ([string]$Name, [string]$DefId, [string]$Value) { + @{ + Basics = @{ Name = $Name; Id = $DefId; Description = ''; ProfileType = 'Test' + Platform = ''; Created = ''; Modified = ''; ScopeTags = ''; Tags = '' } + Settings = @(@{ Name = $Name; SettingPath = $Name; Value = $Value; DefinitionId = $DefId }) + Assignments = @() + PolicyTypeId = 10 + } + } + function script:New-TestModel ([string]$TenantName, [string]$Value) { + @{ + TenantName = $TenantName + TenantId = 1 + Products = [ordered]@{ + 'Intune' = @{ + Categories = [ordered]@{ + 'Windows / Configuration Profiles' = @(script:New-TestPolicy 'WinSetting' 'def-win' $Value) + 'macOS / Configuration Profiles' = @(script:New-TestPolicy 'MacSetting' 'def-mac' $Value) + 'iOS/iPadOS / App Protection Policies' = @(script:New-TestPolicy 'IosSetting' 'def-ios' $Value) + } + } + } + } + } + function script:Get-CategoryKeys ($Result) { + $keys = [System.Collections.Generic.List[string]]::new() + foreach ($p in $Result.Products.Keys) { + foreach ($c in $Result.Products[$p].Categories.Keys) { [void]$keys.Add("$p / $c") } + } + $keys + } + } + + It 'removes nothing when -ExcludeOS is not passed' { + $r = & (Get-Module InforcerCommunity) { + param($s, $d) Compare-InforcerDocModels -SourceModel $s -DestinationModel $d + } (script:New-TestModel 'Src' 'A') (script:New-TestModel 'Dst' 'A') + (script:Get-CategoryKeys $r).Count | Should -Be 3 + } + + It 'removes a category whose KEY contains the excluded OS' { + $r = & (Get-Module InforcerCommunity) { + param($s, $d) Compare-InforcerDocModels -SourceModel $s -DestinationModel $d -ExcludeOS @('macOS') + } (script:New-TestModel 'Src' 'A') (script:New-TestModel 'Dst' 'A') + $keys = script:Get-CategoryKeys $r + $keys | Should -Not -Contain 'Intune / macOS / Configuration Profiles' + $keys | Should -Contain 'Intune / Windows / Configuration Profiles' -Because 'only the named OS goes' + } + + It 'removes several OSes at once' { + $r = & (Get-Module InforcerCommunity) { + param($s, $d) Compare-InforcerDocModels -SourceModel $s -DestinationModel $d -ExcludeOS @('macOS', 'iOS') + } (script:New-TestModel 'Src' 'A') (script:New-TestModel 'Dst' 'A') + $keys = script:Get-CategoryKeys $r + @($keys).Count | Should -Be 1 + $keys | Should -Contain 'Intune / Windows / Configuration Profiles' + } + + It 'still matches a PRODUCT name, which was the only thing that ever worked' { + $r = & (Get-Module InforcerCommunity) { + param($s, $d) Compare-InforcerDocModels -SourceModel $s -DestinationModel $d -ExcludeOS @('Intune') + } (script:New-TestModel 'Src' 'A') (script:New-TestModel 'Dst' 'A') + (script:Get-CategoryKeys $r).Count | Should -Be 0 -Because 'excluding the product drops all its categories' + } + } + + Context 'The destination inherits -SourceBaselineId' { + # Scoping one side only compared N baseline policies against the destination's whole estate, + # so every destination-only policy counted as a deviation: 0.2% where Inforcer says 100%. + BeforeAll { + $script:MinimalDocData = @{ TenantId = 1; TenantName = 'T'; Policies = @(@{ id = 'p1' }) } + } + + It 'scopes the destination to the source baseline when no destination baseline is given' { + $calls = & (Get-Module InforcerCommunity) { + param($docData) + $script:seen = [System.Collections.Generic.List[string]]::new() + Mock Get-InforcerDocData { $docData } -ModuleName InforcerCommunity + Mock Select-InforcerBaselinePolicies { [void]$script:seen.Add($BaselineId); 'BaselineName' } -ModuleName InforcerCommunity + Mock Write-Host {} -ModuleName InforcerCommunity + $null = Get-InforcerComparisonData -SourceTenantId 1 -DestinationTenantId 2 -SourceBaselineId 'Tier 0' + $script:seen + } $script:MinimalDocData + @($calls).Count | Should -Be 2 -Because 'both sides must be scoped' + $calls[0] | Should -Be 'Tier 0' + $calls[1] | Should -Be 'Tier 0' -Because 'the destination inherits the source baseline' + } + + It 'does not override an explicit -DestinationBaselineId' { + $calls = & (Get-Module InforcerCommunity) { + param($docData) + $script:seen = [System.Collections.Generic.List[string]]::new() + Mock Get-InforcerDocData { $docData } -ModuleName InforcerCommunity + Mock Select-InforcerBaselinePolicies { [void]$script:seen.Add($BaselineId); 'BaselineName' } -ModuleName InforcerCommunity + Mock Write-Host {} -ModuleName InforcerCommunity + $null = Get-InforcerComparisonData -SourceTenantId 1 -DestinationTenantId 2 ` + -SourceBaselineId 'Tier 0' -DestinationBaselineId 'Tier 2' + $script:seen + } $script:MinimalDocData + $calls[0] | Should -Be 'Tier 0' + $calls[1] | Should -Be 'Tier 2' + } + + # An INHERITED baseline that will not resolve is legitimate — the destination simply may not + # be a member — so it falls back to the full policy set with a warning. An EXPLICIT one that + # fails is still an error. The mock fails only the second call, which is the destination. + It 'warns and falls back when the destination is not a member of the inherited baseline' { + $r = & (Get-Module InforcerCommunity) { + param($docData) + $script:n = 0 + Mock Get-InforcerDocData { $docData } -ModuleName InforcerCommunity + Mock Select-InforcerBaselinePolicies { + $script:n++ + if ($script:n -eq 1) { 'SourceBaseline' } else { $null } + } -ModuleName InforcerCommunity + Mock Write-Host {} -ModuleName InforcerCommunity + $out = Get-InforcerComparisonData -SourceTenantId 1 -DestinationTenantId 2 -SourceBaselineId 'Tier 0' -WarningVariable w -WarningAction SilentlyContinue -ErrorVariable e -ErrorAction SilentlyContinue + @{ Result = $out; Warnings = $w; Errors = $e } + } $script:MinimalDocData + + $r.Warnings | Should -Not -BeNullOrEmpty -Because 'the fallback must announce that the score is understated' + ($r.Warnings -join ' ') | Should -Match 'full policy set' + $r.Errors | Should -BeNullOrEmpty -Because 'a non-member destination is a legitimate comparison, not an error' + $r.Result | Should -Not -BeNullOrEmpty -Because 'it must still return data to compare' + } + + It 'errors when an EXPLICIT -DestinationBaselineId cannot be resolved' { + $r = & (Get-Module InforcerCommunity) { + param($docData) + $script:n = 0 + Mock Get-InforcerDocData { $docData } -ModuleName InforcerCommunity + Mock Select-InforcerBaselinePolicies { + $script:n++ + if ($script:n -eq 1) { 'SourceBaseline' } else { $null } + } -ModuleName InforcerCommunity + Mock Write-Host {} -ModuleName InforcerCommunity + $out = Get-InforcerComparisonData -SourceTenantId 1 -DestinationTenantId 2 -SourceBaselineId 'Tier 0' -DestinationBaselineId 'Tier 2' -ErrorVariable e -ErrorAction SilentlyContinue + @{ Result = $out; Errors = $e } + } $script:MinimalDocData + + $r.Errors | Should -Not -BeNullOrEmpty -Because 'an explicitly named baseline that does not resolve is a mistake worth surfacing' + "$($r.Errors[0].FullyQualifiedErrorId)" | Should -Match 'DestBaselineFilterFailed' + $r.Result | Should -BeNullOrEmpty + } + } +} From f99cf58c6cac97506382d139467f918000c070ae Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:34:02 +0200 Subject: [PATCH 17/20] docs: document Get-InforcerAuditEvent -Id and -CorrelationId MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both have existed since the parameters were added and are described in Get-Help, but the CMDLET-REFERENCE parameter table skipped them — so the reference listed 8 of the cmdlet's 10 parameters. Found by diffing every cmdlet's real parameter set against its section in the reference, in both directions. Pre-existing on main; not introduced by this release. Co-Authored-By: Claude Opus 5 (1M context) --- docs/CMDLET-REFERENCE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/CMDLET-REFERENCE.md b/docs/CMDLET-REFERENCE.md index 3e92939..06f3ee5 100644 --- a/docs/CMDLET-REFERENCE.md +++ b/docs/CMDLET-REFERENCE.md @@ -333,6 +333,8 @@ Retrieves audit events from the Inforcer API. Supports optional `-EventType`, `- | Parameter | Type | Mandatory | Description | |-----------|------|-----------|-------------| +| **Id** | String | No | Retrieve a specific audit event by its event ID (GUID). Searches all event types and filters client-side. | +| **CorrelationId** | String | No | Retrieve all audit events sharing this correlation ID. Searches all event types and filters client-side. | | **EventType** | String[] | No | Event types to include. Tab completion with supported event types. Omit for all types. | | **DateFrom** | DateTime | No | Start of date/time range (inclusive). | | **DateTo** | DateTime | No | End of date/time range (inclusive). | From 86861def74237443f5d82994de0107f6d90b8244 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:48:21 +0200 Subject: [PATCH 18/20] docs: repair two truncated -Show comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both read "…(Test-InforcerInteractiveHost). Doing it" followed by an orphaned fragment — "run, including inside CI containers…" and "containers, where there is nothing to open it with." Self-inflicted in 321507e: a scripted string replacement swapped the first line of a two-line comment and left the second line dangling. Caught in PR review. Rewritten as complete sentences that state the current behaviour rather than the old rationale. No code change; 434 tests still pass. Co-Authored-By: Claude Opus 5 (1M context) --- module/Public/Compare-InforcerEnvironments.ps1 | 4 ++-- module/Public/Export-InforcerTenantDocumentation.ps1 | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/module/Public/Compare-InforcerEnvironments.ps1 b/module/Public/Compare-InforcerEnvironments.ps1 index 9cd8450..27ffdd7 100644 --- a/module/Public/Compare-InforcerEnvironments.ps1 +++ b/module/Public/Compare-InforcerEnvironments.ps1 @@ -350,8 +350,8 @@ $fileInfo = Get-Item -LiteralPath $filePath $sizeKb = [math]::Round($fileInfo.Length / 1KB, 1) Write-Host " Exported: $filePath ($sizeKb KB)" -ForegroundColor Green -# -Show defaults to on interactively, off in CI (Test-InforcerInteractiveHost). Doing it -# run, including inside CI containers where there is nothing to open it with. +# -Show defaults to on interactively and off in CI (Test-InforcerInteractiveHost), so a human +# who asked for a report sees it while a build agent is never handed a browser to open. if ($Show) { $fullPath = (Resolve-Path -LiteralPath $filePath).Path if ($IsMacOS) { Start-Process 'open' -ArgumentList $fullPath } diff --git a/module/Public/Export-InforcerTenantDocumentation.ps1 b/module/Public/Export-InforcerTenantDocumentation.ps1 index 8e9183f..c174391 100644 --- a/module/Public/Export-InforcerTenantDocumentation.ps1 +++ b/module/Public/Export-InforcerTenantDocumentation.ps1 @@ -451,8 +451,8 @@ foreach ($fmt in $Format) { $fileInfo } -# -Show defaults to on interactively, off in CI (Test-InforcerInteractiveHost). Doing it -# containers, where there is nothing to open it with. +# -Show defaults to on interactively and off in CI (Test-InforcerInteractiveHost), so a human +# who asked for a report sees it while a build agent is never handed a browser to open. if ($Show) { $htmlFile = $Format | Where-Object { $_ -eq 'Html' } | ForEach-Object { if ($Format.Count -eq 1 -and [System.IO.Path]::HasExtension($OutputPath)) { $OutputPath } From 2662bfc2d66fa3c9a8c0c150582c03ee4ddbda89 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:54:15 +0200 Subject: [PATCH 19/20] fix(test): CI-detection tests failed on CI, because they ran on CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite runs on GitHub Actions, which sets GITHUB_ACTIONS=true. The tests manipulated one detector variable at a time and left the runner's own in place, so Test-InforcerInteractiveHost kept returning false no matter what they set. Two consequences, one visible and one not: - "ignores an empty CI variable" FAILED — it cleared CI, GITHUB_ACTIONS was still set, the helper correctly said false, and the test wanted UserInteractive. Green locally, red on the runner. - The seven "returns false when is set" tests PASSED for the wrong reason. Any of them would have passed with the variable under test ignored entirely, because the runner's own variable was doing the work. All of them now clear every detector first and set only the one under test, so a false result can only come from that variable. Adds a third case for "no CI variable set at all". The production code was never wrong — the helper behaved correctly in both environments the whole time. Verified in both: 435 pass / 0 fail with GITHUB_ACTIONS+CI set, and with every detector cleared. Mutation-verified: making the helper always return true now fails all 7 detector tests, where previously it would have failed none of them on a CI runner. Co-Authored-By: Claude Opus 5 (1M context) --- Tests/Consistency.Tests.ps1 | 51 ++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/Tests/Consistency.Tests.ps1 b/Tests/Consistency.Tests.ps1 index 2fb1961..1c1ce6f 100644 --- a/Tests/Consistency.Tests.ps1 +++ b/Tests/Consistency.Tests.ps1 @@ -2258,25 +2258,52 @@ Describe 'Writing files and opening a browser stay opt-in (0.7.0 breaking change # -Show auto-detects: on for a human, off on a build agent. The detection is the whole # safety story now that auto-open is back, so it gets tested directly rather than through # the cmdlets (which would need a live tenant and two minutes per case). + # + # These tests MUST neutralise every detector variable before exercising one, because the suite + # itself runs on a build agent. Setting only the variable under test leaves the runner's own + # GITHUB_ACTIONS=true in place: the "false" assertions then pass for the wrong reason, and the + # empty-value assertion fails outright. That is exactly how this failed in CI the first time. + BeforeAll { + $script:CiVars = 'CI', 'TF_BUILD', 'GITHUB_ACTIONS', 'GITLAB_CI', 'JENKINS_URL', 'TEAMCITY_VERSION', 'BUILDKITE' + function script:Invoke-WithCiEnv { + param([hashtable]$Set, [scriptblock]$Body) + $saved = @{} + foreach ($v in $script:CiVars) { + $saved[$v] = [Environment]::GetEnvironmentVariable($v) + [Environment]::SetEnvironmentVariable($v, $null) + } + try { + foreach ($k in $Set.Keys) { [Environment]::SetEnvironmentVariable($k, $Set[$k]) } + & $Body + } finally { + foreach ($v in $script:CiVars) { [Environment]::SetEnvironmentVariable($v, $saved[$v]) } + } + } + } + It 'Test-InforcerInteractiveHost returns false when is set' -ForEach @( @{ Var = 'CI' }, @{ Var = 'TF_BUILD' }, @{ Var = 'GITHUB_ACTIONS' } @{ Var = 'GITLAB_CI' }, @{ Var = 'JENKINS_URL' }, @{ Var = 'TEAMCITY_VERSION' }, @{ Var = 'BUILDKITE' } ) { - $saved = [Environment]::GetEnvironmentVariable($Var) - try { - [Environment]::SetEnvironmentVariable($Var, 'true') - & (Get-Module InforcerCommunity) { Test-InforcerInteractiveHost } | - Should -BeFalse -Because "a build agent must never have a browser launched at it" - } finally { [Environment]::SetEnvironmentVariable($Var, $saved) } + # Only $Var is set — every other detector is cleared, so a false result can only come from + # this one variable. Without the clearing this assertion would be vacuous on any CI runner. + script:Invoke-WithCiEnv -Set @{ $Var = 'true' } -Body { + & (Get-Module InforcerCommunity) { Test-InforcerInteractiveHost } + } | Should -BeFalse -Because "a build agent must never have a browser launched at it" } It 'Test-InforcerInteractiveHost ignores an empty CI variable' { - $saved = $env:CI - try { - $env:CI = '' - & (Get-Module InforcerCommunity) { Test-InforcerInteractiveHost } | - Should -Be ([Environment]::UserInteractive) -Because 'an empty value is not "in CI"' - } finally { $env:CI = $saved } + # With every detector cleared and CI set to empty string, the answer must fall through to + # UserInteractive — an empty value is not "in CI". + script:Invoke-WithCiEnv -Set @{ 'CI' = '' } -Body { + & (Get-Module InforcerCommunity) { Test-InforcerInteractiveHost } + } | Should -Be ([Environment]::UserInteractive) -Because 'an empty value is not "in CI"' + } + + It 'Test-InforcerInteractiveHost returns true when no CI variable is set at all' { + script:Invoke-WithCiEnv -Set @{} -Body { + & (Get-Module InforcerCommunity) { Test-InforcerInteractiveHost } + } | Should -Be ([Environment]::UserInteractive) -Because 'with nothing set it is the host that decides' } It ' resolves -Show from the interactive check, not a hardcoded default' -ForEach $script:OptInCmdlets { From 1a6c7db526192dd0d81345c6a261525c745dbfa4 Mon Sep 17 00:00:00 2001 From: Roy Klooster <76492458+royklo@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:56:23 +0200 Subject: [PATCH 20/20] docs: note that the upstream schema now contradicts the runtime on membershipRule The 2026-09-04 drift sync merged from main adds membershipRule to tenantGroupSummary in openapi-snapshot.json, described as "only populated for groups whose groupTypes contains DynamicMembership". That is not what the endpoint does. Verified live on 2026-09-09 against two unrelated tenants: 0 of 10 and 0 of 7 DynamicMembership groups carried a rule on the list, while the by-ID endpoint returned one for each. The reference already said "always null on this endpoint"; it now says so against the schema explicitly, with both sample sizes and a pointer to API feedback item 26. A reader diffing the snapshot will otherwise reasonably conclude our note is stale. Co-Authored-By: Claude Opus 5 (1M context) --- docs/API-REFERENCE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/API-REFERENCE.md b/docs/API-REFERENCE.md index 36e490a..1c297d9 100644 --- a/docs/API-REFERENCE.md +++ b/docs/API-REFERENCE.md @@ -740,7 +740,7 @@ Summary of an Entra ID group (returned from list endpoint). | description | string | No | Description of the group. | | mail | string | No | Email address of the group. | | visibility | string | No | Visibility (e.g. Public, Private). | -| membershipRule | string | No | Dynamic membership rule. **Always `null` on this endpoint** — the key is emitted but never filled, including for groups whose `groupTypes` contains `DynamicMembership`. Verified against a live tenant 2026-09-09. Use the by-ID endpoint to read the rule. | +| membershipRule | string | No | Dynamic membership rule. **Always `null` on this endpoint**, despite the upstream OpenAPI schema (added by the 2026-09-04 drift sync in #39) describing it as *"only populated for groups whose groupTypes contains DynamicMembership"*. The key is emitted and never filled — verified live on 2026-09-09 against two unrelated tenants, where 0 of 10 and 0 of 7 `DynamicMembership` groups carried a rule on the list while the by-ID endpoint returned one for each. Use the by-ID endpoint to read the rule. See API feedback item 26. | | groupTypes | array\ | Yes | Types of the group (e.g. Unified, DynamicMembership). | **PSTypeName:** `InforcerCommunity.GroupSummary`