Repository navigation
fix(filters): name the escaped quote, the whitespace -contains and the exclude-mode effect - #33
Conversation
…e exclude-mode effect The filter parser refused a double quote inside a value with "expected and, or or ) ... found hi\" which reads as a missing connector; the service has no escape (FLT-V44, FLT-V45), and the message now says so and points at -contains or -startsWith on the parts around the quote. A -contains value that is only whitespace matched every device in the probe (FLT-W27, FLT-Y03) and the evaluator agreed, with no warning; it now carries an AlwaysMatches warning like -ne with a value no device reports. In exclude mode "never matches" meant the filter excludes nobody and the assignment reaches every device in the group (W32-FILTER-EXCLUDE), and the message did not say so. Get-IslFilterWarningMessage appends the exclude-mode consequence for Test-IntuneAssignmentFilter (-Mode) and Test-IntuneDeployedScript (the assignment filter type); include mode and the other warning kinds are unchanged. The tenant-wide help example keeps to windows10AndLater, since the parser carries the Windows property set and value tables.
fadwen
left a comment
There was a problem hiding this comment.
Notes on the lines whose reason the diff does not show.
| function Test-EscapedQuote { | ||
| param($Previous, $Next) | ||
| if (-not $Previous -or -not $Next -or $Previous.Type -ne 'string') { return $false } | ||
| $glued = $Next.Position -eq $Previous.End + 1 |
There was a problem hiding this comment.
Glued means no whitespace between the two tokens: Position is 1-based and End is the 0-based exclusive end of the previous match, so End + 1 is the position right after it. The existing refusal (device.deviceName -eq "ab" "cd") has a space between the strings and keeps its old message; only the two shapes a would-be escape produces (\" leaves a backslash on the string, "" leaves two strings touching) are renamed.
| if ($next -and $next.Type -eq 'comma') { $state.Pos++ } | ||
| elseif ($next -and $next.Type -ne 'rbracket') { | ||
| Write-Failure "expected ',' or ']' at position $($next.Position), found '$($next.Text)'" | ||
| if (Test-EscapedQuote -Previous $item -Next $next) { Write-EscapeFailure -Previous $item } |
There was a problem hiding this comment.
Inside a list the same shapes end up in the "expected ',' or ']'" branch rather than the connector branches below, so the check is repeated here. [""] stays accepted: an empty string token is a normal item (FLT-W06), and nothing is glued to it.
| } | ||
| } | ||
| } | ||
| if ($Operator -eq 'contains' -and $kind -eq 'String' -and "$value".Trim().Length -eq 0) { |
There was a problem hiding this comment.
Only -contains gets the warning. -startsWith " " would match every device for the same reason (the evaluator trims the value and every string starts with the empty string), but the probe never sent it, so it stays silent until it is measured. -eq " " and the other operators compare a trimmed empty value against the device's and are not affected; the "does not warn" table covers -eq " ".
| [string]$Mode = 'Include' | ||
| ) | ||
|
|
||
| $note = if ($Mode -ne 'Exclude') { '' } |
There was a problem hiding this comment.
A separate helper rather than a -Mode parameter on the parser, because the parser is also what Test-IntuneDeployedScript calls once per filter while the mode belongs to each assignment: one filter can hang off an include assignment and an exclude one at the same time, and the note has to differ per assignment, not per parse.
| ConvertTo-PolicyFinding @unreadSplat | ||
| continue | ||
| } | ||
| $mode = if ($filterType -eq 'exclude') { 'Exclude' } else { 'Include' } |
There was a problem hiding this comment.
Severity is left on the warning kind, not on the mode: an exclude filter whose clause never matches still does nothing, which is the Warning case. The message carries the consequence (reaches every device in the group) so the reader does not have to work it out from the filter type in the label.
| It 'says that a never-matching clause on an exclude filter reaches every device in the group' { | ||
| # Widget 1.0 carries the lab-devices filter in exclude mode; point it at the x64 one | ||
| $target = $script:Tenant.apps[1].assignments[0].target | ||
| $target.deviceAndAppManagementAssignmentFilterId = 'flt-x64' |
There was a problem hiding this comment.
The fixture's one exclude assignment points at the lab-devices filter, which produces no warning, so the test redirects it to the x64 filter and restores it in finally. The tenant hashtable is shared across the file, hence the restore rather than a second fixture.
| @{ Rule = '(device.deviceName -eq "say \"hi\"")'; Message = '*cannot be escaped*position 24*' } | ||
| @{ Rule = '(device.deviceName -eq "say ""hi""")'; Message = '*cannot be escaped*position 24*' } | ||
| @{ Rule = 'device.deviceName -eq "say \"hi\""'; Message = '*cannot be escaped*position 23*' } | ||
| @{ Rule = '(device.deviceName -in ["say \"hi\""])'; Message = '*cannot be escaped*position 25*' } |
There was a problem hiding this comment.
Positions in these four rows are the position of the string token that holds the first half of the broken value, which is where the reader has to look; the earlier found 'hi\' message pointed at the fragment after the quote instead.
| Every filter in the tenant whose rule can never match a Windows device (a "x64" architecture, | ||
| a "Microsoft Entra joined" trust type), without evaluating anything. | ||
| Every Windows filter in the tenant whose rule can never match a Windows device (a "x64" | ||
| architecture, a "Microsoft Entra joined" trust type), without evaluating anything. The platform |
There was a problem hiding this comment.
The reason is spelled out because the obvious guess (non-Windows filters fail to parse) is mostly wrong: an iOS filter on osVersion or enrollmentProfileName parses fine. The hazard is a false "never matches" on a correct macOS value such as x64, which the Windows value table does not contain.
Summary
Four gaps in the assignment filter checks, found while writing up
Test-IntuneAssignmentFilteragainst the probe results in Validation/Findings.md. None changes a verdict; each one changes what the command says about a rule the service accepts.(device.deviceName -contains " ")matched every device in the probe (FLT-W27,FLT-Y03) and the evaluator agreed, but gave no warning, unlike-ne "x64", which gets "matches every device"."say \"hi\""or"say ""hi""", was refused withexpected 'and', 'or' or ')' ... found 'hi\', which reads as a missing connector. The service has no escape (FLT-V44,FLT-V45).Changes
Private/ConvertFrom-IslFilterRule.ps1: a-containsvalue that is only whitespace adds an AlwaysMatches warning. A string token glued to a word ending in a backslash, or two string tokens glued together, where a connector was expected is reported as an escaped quote with no escape, pointing at-containsor-startsWith; the same check covers a list item. Other shapes keep the previous messages.Private/Get-IslFilterWarningMessage.ps1(new): appends the exclude-mode consequence to a NeverMatches or AlwaysMatches warning and passes everything else through, so the two commands say the same thing.Public/Test-IntuneAssignmentFilter.ps1andPublic/Test-IntuneDeployedScript.ps1: warnings go through the helper with the command's-Modeor the assignment's filter type.windows10AndLaterand says why;-Modedescribes the note; theIslFilterIssuerow in the tenant command's help lists the whitespace case and the mode-aware message. MAML rebuilt.CHANGELOG.md: Added and Changed entries under Unreleased.Verification
ConvertFrom-IslFilterRule,Get-IslFilterWarningMessage,Test-IntuneAssignmentFilter,Test-IntuneDeployedScriptandModule.Contract: 264 passed on PowerShell 7.6.6; the first four on Windows PowerShell 5.1: 248 passed. Full unit suite on PowerShell 7.6.6: see the gate run.Build/Build-Help.ps1 -SkipUpdate: help valid, MAML rebuilt and committed. PSScriptAnalyzer (Error, Warning) clean on the four source files; no line over 115 characters.FLT-W27,FLT-Y03,FLT-V44,FLT-V45, W32-FILTER-INCLUDE, W32-FILTER-EXCLUDE).Notes
-startsWithvalue that is only whitespace would match every device the same way, but the probe did not send one, so it gets no warning until it is measured.