From 14daba82ae421bda5f873e7bb5c8aece9753b00c Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Thu, 17 Sep 2026 12:30:54 -0500 Subject: [PATCH 01/10] Fix ChatType and IncludeBCCInGroupChat handling in Send-ScriptMessage - The defaults block tested the parameters for truthiness, so an explicit -ChatType OneOnOne or -IncludeBCCInGroupChat $false was replaced by the configuration file's value. OneOnOne is ChatType enum value 0 and $false is a valid choice, so neither can be told apart from an unsupplied value that way. Test $PSBoundParameters.ContainsKey() instead. - The same block cast the configuration value to [ChatType] unconditionally, so a configuration file with no ChatType setting made every send throw "Cannot convert null to type ChatType", including a Mail-only send that has no use for a chat type. - Resolve both settings into separate $Resolved variables. A parameter keeps its type constraint through later assignments, so [ChatType] cannot be reassigned the $null of a missing setting. This also stops the values leaking between services in a multi-service call, since each iteration now derives them from its own service configuration. - Add ChatType to the Send-ScriptMessage_MicrosoftGraph splat only when [string]::IsNullOrWhiteSpace says there is a real value. Neither $null nor the empty string a configuration file uses for an unset setting can bind to an enum parameter. - Report a Chat message that arrived without a chat type from Send-ScriptMessage_MicrosoftGraph, after its AllowableMessageTypes check, as a "Chat not sent." warning in the result's Error property. Raising it from Send-ScriptMessage instead preempts that check and tells a Mail-only configuration to add ChatType when what it actually needs is Chat in AllowableMessageTypes, and every other chat failure is already reported this way rather than thrown. - Add Pester coverage for both parameters against a configuration that disagrees, the configuration-default path, a Mail-only send with no ChatType setting, a Chat send with no chat type anywhere, a blank ChatType setting, and a Mail-only configuration asked to send Chat, which must still name the disallowed type. The existing chat tests now pass -ChatType rather than setting it in the mocked configuration. - Record the enum and type-constraint measurements behind the fix in Research_Notes/PowerShell-Language-Behavior.md section 10. --- CHANGELOG.md | 3 + .../PowerShell-Language-Behavior.md | 57 ++++- ScriptMessage/Public/Send-ScriptMessage.ps1 | 35 ++- ScriptMessage/Services/MicrosoftGraph.ps1 | 6 + Tests/Send-ScriptMessage.Tests.ps1 | 221 +++++++++++++++++- 5 files changed, 305 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f17c4c9..e7f0e5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ ### Fixed +- `Send-ScriptMessage` now honors an explicit `-ChatType OneOnOne`. The configuration file's `ChatType` setting was used in its place, so a call asking for one-on-one chats sent a single group chat when that setting was `Group`. An explicit `-ChatType Group` was unaffected. +- `Send-ScriptMessage` now honors an explicit `-IncludeBCCInGroupChat $false`. The configuration file's `IncludeBCCInGroupChat` setting was used in its place, so a call asking to keep BCC recipients out of a group chat added them anyway when that setting was `true`, making those addresses visible to everyone in the chat. An explicit `-IncludeBCCInGroupChat $true` was unaffected. +- `Send-ScriptMessage` now sends mail without a `ChatType` setting in the configuration file. A Mail-only send failed without it, reporting "Cannot convert null to type ChatType", even though `ChatType` only affects chat; a configuration file listing only `Mail` in `AllowableMessageTypes`, or one written before the setting was added in version 1.0.8, was affected. A `Chat` send with no chat type, because the setting is missing or blank and `-ChatType` was not passed, now reports that in the returned result's `Error` property and names both places it can be set. - Minor: The help for `Get-ScriptMessageConfig` and `Set-ScriptMessageConfigFilePath` now describes the `-Path` parameter, which it left blank. - Minor: The fourth `Send-ScriptMessage` help example, which sends attachments held in variables, now runs as written. It failed with "Missing closing ')' in subexpression" when copied, and it ended without calling `Send-ScriptMessage`. diff --git a/Research_Notes/PowerShell-Language-Behavior.md b/Research_Notes/PowerShell-Language-Behavior.md index 908c3a0..2ade7ae 100644 --- a/Research_Notes/PowerShell-Language-Behavior.md +++ b/Research_Notes/PowerShell-Language-Behavior.md @@ -39,8 +39,10 @@ passed". `$PSBoundParameters.ContainsKey()` is the test that tells them apart. A three states (unset, true, false) should be `[Nullable[bool]]`. **From source:** `Send-ScriptMessage` fills in `ChatType` and `IncludeBCCInGroupChat` from the configuration -file using exactly this `-not` test, which is why an explicit `-ChatType OneOnOne` or -`-IncludeBCCInGroupChat $false` is replaced by the configuration file's value. +file when the caller did not supply them, and tests `$PSBoundParameters.ContainsKey()` for exactly this reason. +The `-not` test would drop an explicit `-ChatType OneOnOne` or `-IncludeBCCInGroupChat $false` in favor of the +configuration file's value. Section 10 covers why the resolved values go into separate variables rather than +back into the parameters. ## 2. Measured: a `[PSCustomObject]` constraint on a variable does not convert a hashtable @@ -235,3 +237,54 @@ result in this section where the editions differ. tested. Truthiness tests such as `if ($One)` or `-not $One`, and other .NET methods taking a string argument, were not. - A script that sets `$OFS` was not tested. + +## 10. Measured: an enum cannot hold `$null`, and a parameter keeps its type constraint + +Measured on **2026-09-17**, same machine and editions as the header. Both editions agreed on every row. + +With `enum CT { OneOnOne; Group }`: + +| Code | Result | +| --- | --- | +| `[CT]$x = $null`, fresh variable, inside a function | throws "Cannot convert null to type" | +| `param([CT]$x)` then `$x = $null` later in the body | throws the same way | +| `param([CT]$x)` then `$x = 'Group'` later in the body | converts; `$x` is `[CT]` | +| `Test-Thing -ChatType $null` against a `[CT]` parameter | throws "Cannot process argument transformation" | +| Splatting `@{ ChatType = $null }` at a `[CT]` parameter | throws the same way | +| Splatting `@{ B = $null }` at a `[bool]` parameter | throws `Cannot convert value "" to type "System.Boolean"` | +| `[bool]$x = $null`, fresh variable, inside a function | converts; `$x` is `False` | +| `[bool]$x = $true` then `$x = $null` later, inside a function | converts; `$x` is `False` | +| Splatting `@{ ChatType = '' }` at a `[CT]` parameter | throws `The identifier name cannot be processed because it is either too similar or identical to the following enumerator names` | +| `[string]::IsNullOrWhiteSpace()` given a `[CT]` value | `False`; the enum converts to its member name first | +| `[bool]` of the string `'false'` | `True`; every non-empty string is truthy | + +Two consequences. A parameter's type constraint is not just an entry gate: it applies to every later assignment +to that variable in the body, so a parameter cannot be reused as a scratch variable for a value its type +cannot hold. And an enum has no representation for "no value", so a missing configuration setting cannot be +carried in an enum-typed variable or passed to an enum-typed parameter; the call has to omit the argument +instead. + +A `[bool]` behaves differently from an enum in the one case that matters: a `[bool]`-constrained variable +inside a function accepts `$null` and becomes `False`, while a `[bool]` parameter rejects it at binding. + +**From source:** `Send-ScriptMessage` resolves `ChatType` and `IncludeBCCInGroupChat` into separate +`$Resolved...` variables rather than assigning back to the parameters, because a parameter cannot be reassigned +a value its type cannot hold. It adds `ChatType` to the `Send-ScriptMessage_MicrosoftGraph` splat only when +`[string]::IsNullOrWhiteSpace` says there is a real value, because neither `$null` nor `''` can bind to the +service's `[ChatType]` parameter, and `Templates/config_scriptmessage.json` writes a setting it leaves unset as +an empty string. `Send-ScriptMessage_MicrosoftGraph` then reports a Chat message that arrived without one. +`IncludeBCCInGroupChat` needs no such guard: it is resolved through a `[bool]`-constrained variable inside a +function, so a configuration file with no `IncludeBCCInGroupChat` setting yields `False`. + +The last row is a trap this module has not addressed: a configuration file that quotes the value, as +`"IncludeBCCInGroupChat": "false"`, yields `True`, because `[bool]` of any non-empty string is `True`. JSON +`false` is unquoted and converts correctly, so only a hand-edited file with the quotes is affected. + +### What this does not establish + +- Why the same `[bool]` assignment that converts `$null` inside a function throws at script scope. Both + editions do it, and the module is unaffected because all of its code lives in functions, but the cause was + not traced. +- `[Nullable[bool]]` and `[CT?]` parameters were not tested here; section 1 covers an unassigned + `[Nullable[bool]]` variable only. +- Only `$null` was tested as the rejected value. Empty strings and other unconvertible values were not. diff --git a/ScriptMessage/Public/Send-ScriptMessage.ps1 b/ScriptMessage/Public/Send-ScriptMessage.ps1 index 25ec3fc..e18bcc5 100644 --- a/ScriptMessage/Public/Send-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Send-ScriptMessage.ps1 @@ -293,14 +293,28 @@ function Send-ScriptMessage ServiceConfig = $ScriptMessageConfig.$($serviceTypeObj.Service) } - # Set default values if not specified by a parameter. - if (-not $ChatType) + # Set default values for anything the caller did not specify. These test the bound parameters rather than + # the values because truthiness cannot tell either parameter apart from an unsupplied one: 'OneOnOne' is + # ChatType enum value 0, and $false is a valid 'IncludeBCCInGroupChat' choice. The results go into + # separate variables because a parameter keeps its type constraint through later assignments, and + # [ChatType] cannot hold the $null of a configuration file that has no 'ChatType' setting. + if ($PSBoundParameters.ContainsKey('ChatType')) { - [ChatType]$ChatType = $ConnectionParameters.ServiceConfig.ChatType + $ResolvedChatType = $ChatType } - if (-not $IncludeBCCInGroupChat) + else { - [bool]$IncludeBCCInGroupChat = $ConnectionParameters.ServiceConfig.IncludeBCCInGroupChat + $ResolvedChatType = $ConnectionParameters.ServiceConfig.ChatType + } + + # 'IncludeBCCInGroupChat' accepts $null to mean "use the configured value". + if ($PSBoundParameters.ContainsKey('IncludeBCCInGroupChat') -and ($null -ne $IncludeBCCInGroupChat)) + { + [bool]$ResolvedIncludeBCCInGroupChat = $IncludeBCCInGroupChat + } + else + { + [bool]$ResolvedIncludeBCCInGroupChat = $ConnectionParameters.ServiceConfig.IncludeBCCInGroupChat } # Connect to the messaging service, if necessary (e.g., API service). @@ -321,8 +335,15 @@ function Send-ScriptMessage Attachment = $Attachment SenderId = $SenderId Type = $serviceTypeObj.Type - ChatType = $ChatType - IncludeBCCInGroupChat = $IncludeBCCInGroupChat + IncludeBCCInGroupChat = $ResolvedIncludeBCCInGroupChat + } + + # [ChatType] cannot be bound to $null, nor to the empty string a configuration file uses for a + # setting it leaves unset, so pass it only when there is a real value. A Mail-only send does not + # need one, and the service reports a Chat message that has none. + if (-not [string]::IsNullOrWhiteSpace($ResolvedChatType)) + { + $SendMessageParameters['ChatType'] = $ResolvedChatType } Send-ScriptMessage_MicrosoftGraph @SendMessageParameters diff --git a/ScriptMessage/Services/MicrosoftGraph.ps1 b/ScriptMessage/Services/MicrosoftGraph.ps1 index 35abe37..9bbd3fd 100644 --- a/ScriptMessage/Services/MicrosoftGraph.ps1 +++ b/ScriptMessage/Services/MicrosoftGraph.ps1 @@ -693,6 +693,12 @@ function Send-ScriptMessage_MicrosoftGraph Write-Warning -Message $NewMessage $MgWarningMessages += "$NewMessage" } + elseif ($null -eq $ChatType) + { + $NewMessage = "Chat not sent. No chat type is set. Add the 'ChatType' setting to the '$ServiceId' section of the configuration file, or use the 'ChatType' parameter." + Write-Warning -Message $NewMessage + $MgWarningMessages += "$NewMessage" + } else { # Grab the latest MicrosoftGraph service context. diff --git a/Tests/Send-ScriptMessage.Tests.ps1 b/Tests/Send-ScriptMessage.Tests.ps1 index 7ec8701..2677b81 100644 --- a/Tests/Send-ScriptMessage.Tests.ps1 +++ b/Tests/Send-ScriptMessage.Tests.ps1 @@ -133,8 +133,6 @@ Describe 'Send-ScriptMessage chat message' { } } - # Each context sets ChatType in the configuration rather than passing -ChatType, because the configuration - # value currently wins over an explicitly passed parameter. Context 'A one-on-one chat with no attachments' { BeforeAll { Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { @@ -142,7 +140,6 @@ Describe 'Send-ScriptMessage chat message' { MicrosoftGraph = [pscustomobject]@{ AllowableMessageTypes = @('Mail', 'Chat') MailType = 'Group' - ChatType = 'OneOnOne' IncludeBCCInGroupChat = $false MgPermissionType = 'Delegated' MgDisconnectWhenDone = $false @@ -154,7 +151,7 @@ Describe 'Send-ScriptMessage chat message' { } It 'sends the message body and reports no error' { - $Result = Send-ScriptMessage @MessageArguments + $Result = Send-ScriptMessage @MessageArguments -ChatType OneOnOne $Result.Error | Should -BeNullOrEmpty $Result.Status.Id | Should -Be 'new-message-id' @@ -171,7 +168,6 @@ Describe 'Send-ScriptMessage chat message' { MicrosoftGraph = [pscustomobject]@{ AllowableMessageTypes = @('Mail', 'Chat') MailType = 'Group' - ChatType = 'Group' IncludeBCCInGroupChat = $false MgPermissionType = 'Delegated' MgDisconnectWhenDone = $false @@ -182,7 +178,7 @@ Describe 'Send-ScriptMessage chat message' { } It 'sends the message body and reports no error' { - $Result = Send-ScriptMessage @MessageArguments + $Result = Send-ScriptMessage @MessageArguments -ChatType Group $Result.Error | Should -BeNullOrEmpty $Result.Status.Id | Should -Be 'new-message-id' @@ -199,7 +195,6 @@ Describe 'Send-ScriptMessage chat message' { MicrosoftGraph = [pscustomobject]@{ AllowableMessageTypes = @('Mail', 'Chat') MailType = 'Group' - ChatType = 'OneOnOne' IncludeBCCInGroupChat = $false MgPermissionType = 'Delegated' MgDisconnectWhenDone = $false @@ -212,6 +207,7 @@ Describe 'Send-ScriptMessage chat message' { It 'sends both, and the chat carries its own message content' { $Arguments = $MessageArguments.Clone() $Arguments['Type'] = @('Mail', 'Chat') + $Arguments['ChatType'] = 'OneOnOne' $Result = @(Send-ScriptMessage @Arguments) @@ -233,7 +229,6 @@ Describe 'Send-ScriptMessage chat message' { MicrosoftGraph = [pscustomobject]@{ AllowableMessageTypes = @('Mail', 'Chat') MailType = 'Group' - ChatType = 'OneOnOne' IncludeBCCInGroupChat = $false MgPermissionType = 'Delegated' MgDisconnectWhenDone = $false @@ -253,6 +248,7 @@ Describe 'Send-ScriptMessage chat message' { It 'escapes the attachment filename for the upload path' { $Arguments = $MessageArguments.Clone() $Arguments['Attachment'] = @{ Name = 'My File+1.pdf'; Content = [byte[]]@(1, 2, 3) } + $Arguments['ChatType'] = 'OneOnOne' $Result = Send-ScriptMessage @Arguments @@ -305,3 +301,212 @@ Describe 'Send-ScriptMessage mail attachment' { } } } + +Describe 'Send-ScriptMessage configuration defaults' { + BeforeAll { + Mock -ModuleName ScriptMessage Connect-ScriptMessage { } + Mock -ModuleName ScriptMessage Get-ScriptMessageContext { + [pscustomobject]@{ + Account = 'sender@example.org' + Scopes = @('Chat.ReadWrite') + } + } + Mock -ModuleName ScriptMessage New-MgChat { [pscustomobject]@{ Id = 'new-chat-id' } } + Mock -ModuleName ScriptMessage New-MgChatMessage { [pscustomobject]@{ Id = 'new-message-id' } } + Mock -ModuleName ScriptMessage Get-MgChat { @() } + Mock -ModuleName ScriptMessage Send-MgUserMail { $true } + + $ChatArguments = @{ + Service = 'MicrosoftGraph' + Type = 'Chat' + From = 'sender@example.org' + To = 'recipient@example.org' + Subject = 'Test subject' + Body = 'Test body' + } + } + + Context 'A configuration that sets ChatType to Group' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail', 'Chat') + MailType = 'Group' + ChatType = 'Group' + IncludeBCCInGroupChat = $false + MgPermissionType = 'Delegated' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + # 'OneOnOne' is the first ChatType enum member, so its numeric value is 0. A truthiness test on the + # parameter cannot tell it apart from an unsupplied value. + It 'an explicit -ChatType OneOnOne wins over the configuration' { + $null = Send-ScriptMessage @ChatArguments -ChatType OneOnOne + + Should -Invoke -ModuleName ScriptMessage New-MgChat -Times 1 -Exactly -ParameterFilter { + $ChatType -eq 'OneOnOne' + } + # Only the Group branch looks for a chat to reuse. + Should -Invoke -ModuleName ScriptMessage Get-MgChat -Times 0 -Exactly + } + + It 'the configuration value is used when -ChatType is not supplied' { + $null = Send-ScriptMessage @ChatArguments + + Should -Invoke -ModuleName ScriptMessage New-MgChat -Times 1 -Exactly -ParameterFilter { + $ChatType -eq 'Group' + } + Should -Invoke -ModuleName ScriptMessage Get-MgChat -Times 1 -Exactly + } + } + + Context 'A configuration that sets IncludeBCCInGroupChat to true' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail', 'Chat') + MailType = 'Group' + ChatType = 'Group' + IncludeBCCInGroupChat = $true + MgPermissionType = 'Delegated' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'an explicit -IncludeBCCInGroupChat $false wins over the configuration' { + $Result = Send-ScriptMessage @ChatArguments -BCC 'blind@example.org' -IncludeBCCInGroupChat $false -WarningAction SilentlyContinue + + @($Result.Error | Where-Object { $_.Type -eq 'Warning' -and $_.Message -like '*not included in the group chat*' }).Count | + Should -Be 1 + } + + It 'the configuration value is used when -IncludeBCCInGroupChat is not supplied' { + $Result = Send-ScriptMessage @ChatArguments -BCC 'blind@example.org' + + $Result.Error | Should -BeNullOrEmpty + } + } + + Context 'A configuration written before the ChatType setting existed' { + BeforeAll { + # A configuration file from 1.0.7 or earlier has no ChatType setting at all. + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + MgPermissionType = 'Application' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'sends Mail without needing a ChatType setting' { + $MailArguments = @{ + Service = 'MicrosoftGraph' + Type = 'Mail' + From = 'sender@example.org' + To = 'recipient@example.org' + Subject = 'Test subject' + Body = 'Test body' + } + + # A missing 'ChatType' must not stop a Mail send, so reaching the assertions at all is half of + # what this checks. + $Result = Send-ScriptMessage @MailArguments -ErrorAction Stop + + # A Mail send has no chat type and must not be told about one. + $Result.Error | Should -BeNullOrEmpty + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly + } + + # The configuration allows Mail only, so that is what the caller has to change. A missing 'ChatType' is + # not the reason this send cannot happen, and saying so would send them to the wrong setting. + It 'reports the disallowed message type rather than the missing ChatType' { + $ChatOnly = @{ + Service = 'MicrosoftGraph' + Type = 'Chat' + From = 'sender@example.org' + To = 'recipient@example.org' + Subject = 'Test subject' + Body = 'Test body' + } + + $Result = Send-ScriptMessage @ChatOnly -WarningAction SilentlyContinue + + @($Result.Error | Where-Object { $_.Message -like '*does not allow sending messages of type*' }).Count | + Should -Be 1 + $Result.Error | Where-Object { $_.Message -like '*No chat type is set*' } | Should -BeNullOrEmpty + } + } + + Context 'A configuration that allows Chat but sets no ChatType' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail', 'Chat') + IncludeBCCInGroupChat = $false + MgPermissionType = 'Delegated' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'reports the missing chat type in the result instead of throwing' { + $Result = Send-ScriptMessage @ChatArguments -WarningAction SilentlyContinue + + @($Result.Error | Where-Object { $_.Message -like '*No chat type is set*' }).Count | Should -Be 1 + Should -Invoke -ModuleName ScriptMessage New-MgChat -Times 0 -Exactly + } + + It 'still sends Mail in the same call' { + $Arguments = $ChatArguments.Clone() + $Arguments['Type'] = @('Mail', 'Chat') + + $Result = @(Send-ScriptMessage @Arguments -WarningAction SilentlyContinue) + + @($Result | Where-Object { $_.MessageType -eq 'Mail' -and $null -eq $_.Error }).Count | Should -Be 1 + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly + } + } + + Context 'A configuration that leaves ChatType blank' { + BeforeAll { + # Templates/config_scriptmessage.json writes a setting it leaves unset as an empty string, so a + # blank 'ChatType' has to read the same as a missing one. + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail', 'Chat') + ChatType = '' + IncludeBCCInGroupChat = $false + MgPermissionType = 'Delegated' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'reports the missing chat type rather than failing to convert an empty string' { + $Result = Send-ScriptMessage @ChatArguments -WarningAction SilentlyContinue + + @($Result.Error | Where-Object { $_.Message -like '*No chat type is set*' }).Count | Should -Be 1 + $Result.Error | Where-Object { $_.Message -like '*identifier name*' } | Should -BeNullOrEmpty + Should -Invoke -ModuleName ScriptMessage New-MgChat -Times 0 -Exactly + } + } +} From f6b1f7e41bc25900e3155ee1482ba4dfdb37cf25 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Thu, 17 Sep 2026 12:39:04 -0500 Subject: [PATCH 02/10] Use RFC 2606 reserved domains for every placeholder - AGENTS.md and CONTRIBUTING.md required placeholder addresses on domain.com, which is a real registered domain. Require a domain reserved by RFC 2606 instead: example.com, example.net, or example.org. None of those can ever be registered, so a placeholder copied out of a help example and run unedited, or a test whose mock is missing, cannot reach a real recipient. The help examples were the weakest spot, since those are what a reader copies and runs. - Send-ScriptMessage help: 14 example addresses to example.com. - Research_Notes/PowerShell-Language-Behavior.md section 9 records observed output, so the expressions were re-run with the new address in both editions rather than edited by hand. - Tests: the one placeholder URL in the attachment mock. - The rule states example.com as the preference but all three as equally correct, so nobody renames the test suite's existing example.org addresses to match. --- AGENTS.md | 7 ++++-- CONTRIBUTING.md | 3 ++- .../PowerShell-Language-Behavior.md | 6 ++--- ScriptMessage/Public/Send-ScriptMessage.ps1 | 24 +++++++++---------- Tests/Send-ScriptMessage.Tests.ps1 | 2 +- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5dc8a3e..1ca8ec5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,8 +19,11 @@ repository (`Sekers/ScriptMessage.wiki`), not part of this one. `Tests/` use whichever configuration their `Set-ScriptMessageConfigFilePath` line names; read that line, and the recipients, before running anything. - **Never name the production tenant in any file.** No organization name, abbreviation, or email domain in - code, comments, help examples, tests, or commit messages. Use placeholder addresses on `domain.com`, as the - existing help examples do. + code, comments, help examples, tests, or commit messages. Every placeholder address and URL uses a domain + reserved by [RFC 2606](https://www.rfc-editor.org/rfc/rfc2606): `example.com`, `example.net`, or + `example.org`. None of them can ever be registered, so a placeholder copied out of a help example and run + unedited, or a test whose mock is missing, cannot reach a real recipient. Prefer `example.com` for anything + new, but all three are equally correct: never rename existing placeholders to match that preference. - **Never commit a real configuration file.** Configuration files hold the tenant ID, client ID, certificate details, and encrypted secrets. `Tests/Config/` and `@Local Only/` are gitignored for that reason, and `Templates/config_scriptmessage.json` must only ever contain placeholder values. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a9a6c5a..7af6c61 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -24,7 +24,8 @@ Thank you for helping improve ScriptMessage. This guide covers the everyday work ## Rules - **Never commit a real configuration file, secret, or certificate.** Never name a real organization, tenant, or - email domain in code, tests, examples, or commit messages; use `domain.com` addresses instead. + email domain in code, tests, examples, or commit messages; use an RFC 2606 reserved domain instead, such as + `example.com`. Those can never be registered, so a placeholder that gets run unedited reaches nobody. - **The automated tests never connect to a tenant.** If a change needs a live Microsoft Graph test, run it in your own test tenant and describe what you tested in the pull request. - **Text files are UTF-8 with LF line endings and no byte order mark, and PowerShell files contain only ASCII.** diff --git a/Research_Notes/PowerShell-Language-Behavior.md b/Research_Notes/PowerShell-Language-Behavior.md index 2ade7ae..dd7e00d 100644 --- a/Research_Notes/PowerShell-Language-Behavior.md +++ b/Research_Notes/PowerShell-Language-Behavior.md @@ -178,12 +178,12 @@ opens the file with .NET, so a configuration file path containing brackets works ## 9. Measured: a `PSCustomObject` in a one-item array converts to an empty string Measured on **2026-09-16** with the same editions, in a standalone script with `$OFS` unset. With -`$Obj = [pscustomobject]@{ Name = 'A'; AddressObj = 'a@domain.com' }`, `$One = @($Obj)`, and +`$Obj = [pscustomobject]@{ Name = 'A'; AddressObj = 'a@example.com' }`, `$One = @($Obj)`, and `$Two = @($Obj, $Obj)`: | Expression | Result | | --- | --- | -| `[string]$Obj` or `"$Obj"` | `@{Name=A; AddressObj=a@domain.com}` | +| `[string]$Obj` or `"$Obj"` | `@{Name=A; AddressObj=a@example.com}` | | `$Obj.ToString()` | an empty string | | `[string]$One` or `"$One"` | an empty string | | `[string]$Two` | a single space | @@ -192,7 +192,7 @@ Measured on **2026-09-16** with the same editions, in a standalone script with ` | `[string]::IsNullOrEmpty($Two)` | `False` (`IsNullOrWhiteSpace` gives `True`) | | `[string]::IsNullOrEmpty($One.AddressObj)` | `False` | | `[string]@(@{ Name = 'x' })`, a hashtable in a one-item array | `System.Collections.Hashtable`, so `IsNullOrEmpty` gives `False` | -| `[string]@('a@domain.com')` | `a@domain.com` | +| `[string]@('a@example.com')` | `a@example.com` | | `[string]::IsNullOrEmpty(@())` | `True` | So `[string]::IsNullOrEmpty` cannot tell an empty array from one holding a single `PSCustomObject`, and for an diff --git a/ScriptMessage/Public/Send-ScriptMessage.ps1 b/ScriptMessage/Public/Send-ScriptMessage.ps1 index e18bcc5..bbcc090 100644 --- a/ScriptMessage/Public/Send-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Send-ScriptMessage.ps1 @@ -58,8 +58,8 @@ function Send-ScriptMessage .EXAMPLE $MessageArguments = @{ - From = 'jdoe@domain.com' - To = 'bmayes@domain.com' + From = 'jdoe@example.com' + To = 'bmayes@example.com' CC = @() Subject = "Test Message" Body = @{ @@ -70,9 +70,9 @@ function Send-ScriptMessage Send-ScriptMessage -Service MicrosoftGraph -Type 'Mail' @MessageArguments .EXAMPLE $MessageArguments = @{ - From = 'jdoe@domain.com' - To = @('bmayes@domain.com') - CC = @( "hcoonly@domain.com", "plittle@domain.com") + From = 'jdoe@example.com' + To = @('bmayes@example.com') + CC = @( "hcoonly@example.com", "plittle@example.com") Subject = "Test Message" Body = @{ Content = "This is a test message.`n`nThank you!" @@ -84,14 +84,14 @@ function Send-ScriptMessage $MessageArguments = @{ From = @{ Name = 'John Doe' - Address = 'jdoe@domain.com' + Address = 'jdoe@example.com' } ReplyTo= @{ Name = "Lisa Maloney" - Address = "lmaloney@domain.com" + Address = "lmaloney@example.com" } - To = @('bmayes@domain.com') - CC = @( "hcoonly@domain.com", "plittle@domain.com") + To = @('bmayes@example.com') + CC = @( "hcoonly@example.com", "plittle@example.com") SaveToSentItems = $true Subject = "Test Message" Body = @{ @@ -99,7 +99,7 @@ function Send-ScriptMessage Content = "This is a test message.`n`nThank you!" } Attachment = @('C:\StuffToSend\', 'C:\Documents\AnotherFile.pdf') - SenderId = 'senderaccount@domain.com' + SenderId = 'senderaccount@example.com' } Send-ScriptMessage -Service MicrosoftGraph @MessageArguments @@ -129,9 +129,9 @@ function Send-ScriptMessage $MessageArguments = @{ From = @{ - Address = 'jdoe@domain.com' + Address = 'jdoe@example.com' } - To = @('bmayes@domain.com') + To = @('bmayes@example.com') Subject = "Test Message" Body = @{ Content = "This is a test message.`n`nThank you!" diff --git a/Tests/Send-ScriptMessage.Tests.ps1 b/Tests/Send-ScriptMessage.Tests.ps1 index 2677b81..2c0d9ab 100644 --- a/Tests/Send-ScriptMessage.Tests.ps1 +++ b/Tests/Send-ScriptMessage.Tests.ps1 @@ -240,7 +240,7 @@ Describe 'Send-ScriptMessage chat message' { Mock -ModuleName ScriptMessage Get-MgDriveItem { $null } # The chat files folder does not exist yet. Mock -ModuleName ScriptMessage Invoke-MgGraphRequest { # Graph returns the uploaded drive item as a hashtable, which the attachment converter reads by key. - @{ id = 'drive-item-id'; name = 'My File+1.pdf'; webUrl = 'https://www.domain.com/file' } + @{ id = 'drive-item-id'; name = 'My File+1.pdf'; webUrl = 'https://www.example.com/file' } } Mock -ModuleName ScriptMessage Invoke-MgInviteDriveItem { @{ id = 'invite-id' } } } From 9a31c0fad351c729cad36fe22705e668d6f33c75 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Thu, 17 Sep 2026 15:25:57 -0500 Subject: [PATCH 03/10] Require boolean configuration settings and parameters to be booleans - [bool] of any non-empty string is $true, so a quoted "false" meant its opposite. IncludeBCCInGroupChat was read with a [bool] cast and MgDisconnectWhenDone with a truthiness test, so both inverted. The two MgDelegatedPermission_* settings are read as -eq $true, which compares as strings and is already correct. - Declare -IncludeBCCInGroupChat as [Nullable[bool]] instead of [Object] with [ValidateSet($null, $true, $false)]. The binder refuses a string outright, while the validation set accepted 'false' and left it a string for the later cast to turn into $true. Nullable keeps the three states the parameter needs, so $null still means "use the configured value", and an unsupplied parameter holds $null too, which retires the $PSBoundParameters.ContainsKey() test. - A configuration file value never passes through a binder, so add Get-ScriptMessageBooleanSetting to reject anything -isnot [bool] with an error naming the setting. Giving that helper a [Nullable[bool]] parameter would delegate the check to the binder but report the internal parameter name instead of the setting, which is no help to someone holding a JSON file. - Each layer checks the settings it owns, before anything is sent: Send-ScriptMessage for the generic IncludeBCCInGroupChat, and Send-ScriptMessage_MicrosoftGraph for MgDisconnectWhenDone. That means moving the Disconnect-MgGraph teardown into the service as well, so the layer that validates the setting is the one that consumes it. - Add Pester coverage for a string passed to the parameter, a quoted value for each setting, and that neither sends. The success cases assert Send-MgUserMail was invoked, because an earlier version of these tests built its config mocks with GetNewClosure(), which swallowed Pester's injected $Service and passed without sending anything. - Record the binder, cast, class property and ValidateSet differences in Research_Notes/PowerShell-Language-Behavior.md section 10. --- CHANGELOG.md | 2 + .../PowerShell-Language-Behavior.md | 28 ++++-- .../Get-ScriptMessageBooleanSetting.ps1 | 33 +++++++ ScriptMessage/Public/Send-ScriptMessage.ps1 | 17 ++-- ScriptMessage/Services/MicrosoftGraph.ps1 | 11 +++ Tests/Send-ScriptMessage.Tests.ps1 | 98 +++++++++++++++++++ 6 files changed, 171 insertions(+), 18 deletions(-) create mode 100644 ScriptMessage/Private/Get-ScriptMessageBooleanSetting.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index e7f0e5e..8c53b12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Fixed +- `Send-ScriptMessage` now requires the `IncludeBCCInGroupChat` setting to be an unquoted `true` or `false`, not a quoted string. Quotes make the value text, and any non-empty text was read as `true`. So a configuration file holding `"IncludeBCCInGroupChat": "false"` added BCC recipients to a group chat and made their addresses visible to everyone in it, and passing the string `'false'` to `-IncludeBCCInGroupChat` did the same. Both now stop with an error naming the setting or the parameter; remove the quotes in the configuration file, and pass `$false` to the parameter. +- `Send-ScriptMessage` now requires the `MgDisconnectWhenDone` setting to be an unquoted `true` or `false`, not a quoted string. Quotes make the value text, which was read as `true`, so a configuration file holding `"MgDisconnectWhenDone": "false"` disconnected from Microsoft Graph after sending, the opposite of what the setting asks for. It now stops with an error naming the setting; remove the quotes. - `Send-ScriptMessage` now honors an explicit `-ChatType OneOnOne`. The configuration file's `ChatType` setting was used in its place, so a call asking for one-on-one chats sent a single group chat when that setting was `Group`. An explicit `-ChatType Group` was unaffected. - `Send-ScriptMessage` now honors an explicit `-IncludeBCCInGroupChat $false`. The configuration file's `IncludeBCCInGroupChat` setting was used in its place, so a call asking to keep BCC recipients out of a group chat added them anyway when that setting was `true`, making those addresses visible to everyone in the chat. An explicit `-IncludeBCCInGroupChat $true` was unaffected. - `Send-ScriptMessage` now sends mail without a `ChatType` setting in the configuration file. A Mail-only send failed without it, reporting "Cannot convert null to type ChatType", even though `ChatType` only affects chat; a configuration file listing only `Mail` in `AllowableMessageTypes`, or one written before the setting was added in version 1.0.8, was affected. A `Chat` send with no chat type, because the setting is missing or blank and `-ChatType` was not passed, now reports that in the returned result's `Error` property and names both places it can be set. diff --git a/Research_Notes/PowerShell-Language-Behavior.md b/Research_Notes/PowerShell-Language-Behavior.md index dd7e00d..b7343f5 100644 --- a/Research_Notes/PowerShell-Language-Behavior.md +++ b/Research_Notes/PowerShell-Language-Behavior.md @@ -39,10 +39,11 @@ passed". `$PSBoundParameters.ContainsKey()` is the test that tells them apart. A three states (unset, true, false) should be `[Nullable[bool]]`. **From source:** `Send-ScriptMessage` fills in `ChatType` and `IncludeBCCInGroupChat` from the configuration -file when the caller did not supply them, and tests `$PSBoundParameters.ContainsKey()` for exactly this reason. -The `-not` test would drop an explicit `-ChatType OneOnOne` or `-IncludeBCCInGroupChat $false` in favor of the -configuration file's value. Section 10 covers why the resolved values go into separate variables rather than -back into the parameters. +file when the caller did not supply them. The `-not` test would drop an explicit `-ChatType OneOnOne` or +`-IncludeBCCInGroupChat $false` in favor of the configuration file's value, so `ChatType` tests +`$PSBoundParameters.ContainsKey()` and `IncludeBCCInGroupChat`, declared `[Nullable[bool]]` as the last line +above recommends, tests `$null -ne`. Section 10 covers why the resolved values go into separate variables +rather than back into the parameters, and why that declaration is what rejects a string. ## 2. Measured: a `[PSCustomObject]` constraint on a variable does not convert a hashtable @@ -257,6 +258,11 @@ With `enum CT { OneOnOne; Group }`: | Splatting `@{ ChatType = '' }` at a `[CT]` parameter | throws `The identifier name cannot be processed because it is either too similar or identical to the following enumerator names` | | `[string]::IsNullOrWhiteSpace()` given a `[CT]` value | `False`; the enum converts to its member name first | | `[bool]` of the string `'false'` | `True`; every non-empty string is truthy | +| `-b 'false'` at a `[bool]` or `[Nullable[bool]]` parameter | throws: "Boolean parameters accept only Boolean values and numbers, such as $True, $False, 1 or 0" | +| `-b 'false'` at an `[Object]` parameter carrying `[ValidateSet($null, $true, $false)]` | passes validation, and `$b` is still the string `'false'` | +| `-b 0` at that same `[ValidateSet]` parameter | throws; the set holds `''`, `True` and `False` as strings | +| `-b 1` at a `[bool]` or `[Nullable[bool]]` parameter | `True` | +| An unsupplied `[Nullable[bool]]` parameter, and `-b $null` | both `$null` | Two consequences. A parameter's type constraint is not just an entry gate: it applies to every later assignment to that variable in the body, so a parameter cannot be reused as a scratch variable for a value its type @@ -276,9 +282,17 @@ an empty string. `Send-ScriptMessage_MicrosoftGraph` then reports a Chat message `IncludeBCCInGroupChat` needs no such guard: it is resolved through a `[bool]`-constrained variable inside a function, so a configuration file with no `IncludeBCCInGroupChat` setting yields `False`. -The last row is a trap this module has not addressed: a configuration file that quotes the value, as -`"IncludeBCCInGroupChat": "false"`, yields `True`, because `[bool]` of any non-empty string is `True`. JSON -`false` is unquoted and converts correctly, so only a hand-edited file with the quotes is affected. +The parameter binder is stricter than a cast or a validation set, and that difference is load-bearing. A +`[bool]` or `[Nullable[bool]]` parameter refuses a string, while `[Object]` with `[ValidateSet($null, $true, +$false)]` accepts `'false'` and leaves it a string for a later `[bool]` cast to turn into `True`. A +configuration file value never passes through a binder at all, so it needs its own check. + +**From source:** `Send-ScriptMessage` declares `-IncludeBCCInGroupChat` as `[Nullable[bool]]`, which keeps the +three states it needs (unset, true, false) and makes the binder reject a string. A configuration file value +reaches no binder, so `Get-ScriptMessageBooleanSetting` rejects any value that is `-isnot [bool]`. Each layer +calls it for the settings it owns and before anything is sent: `Send-ScriptMessage` for the generic +`IncludeBCCInGroupChat`, and `Send-ScriptMessage_MicrosoftGraph` for its own `MgDisconnectWhenDone`, ahead of +that function's message type loop. Unquoted JSON `true` and `false` arrive as real booleans and are unaffected. ### What this does not establish diff --git a/ScriptMessage/Private/Get-ScriptMessageBooleanSetting.ps1 b/ScriptMessage/Private/Get-ScriptMessageBooleanSetting.ps1 new file mode 100644 index 0000000..d0db95b --- /dev/null +++ b/ScriptMessage/Private/Get-ScriptMessageBooleanSetting.ps1 @@ -0,0 +1,33 @@ +function Get-ScriptMessageBooleanSetting +{ + [CmdletBinding()] + param( + [Parameter( + Mandatory = $true, + ValueFromPipeline = $true, + ValueFromPipelineByPropertyName = $true)] + [AllowNull()] + [Object]$Value, + + [Parameter( + Mandatory = $true, + ValueFromPipelineByPropertyName = $true)] + [string]$Name + ) + + # An unset setting stays unset. The caller decides what that means for its own setting. + if ($null -eq $Value) + { + return $null + } + + # A JSON boolean arrives as a [bool]. A quoted "true" or "false" arrives as a string, and casting a string + # to [bool] makes every non-empty one $true, so a quoted "false" would mean its opposite. Reject anything + # that is not already a boolean rather than guessing at it. + if ($Value -isnot [bool]) + { + throw "The `'$Name`' setting in the ScriptMessage configuration file must be a boolean written as true or false without quotes. Found the $($Value.GetType().Name) value `'$Value`'." + } + + return $Value +} diff --git a/ScriptMessage/Public/Send-ScriptMessage.ps1 b/ScriptMessage/Public/Send-ScriptMessage.ps1 index bbcc090..7ee0bc2 100644 --- a/ScriptMessage/Public/Send-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Send-ScriptMessage.ps1 @@ -246,8 +246,7 @@ function Send-ScriptMessage Mandatory = $false, ValueFromPipeline = $true, ValueFromPipelineByPropertyName = $true)] - [ValidateSet($null, $true, $false)] - [Object]$IncludeBCCInGroupChat # Is an object so it can be set to $null + [Nullable[bool]]$IncludeBCCInGroupChat # Nullable so it can be set to $null, meaning "use the configured value" ) # Set the necessary configuration variables. @@ -307,14 +306,16 @@ function Send-ScriptMessage $ResolvedChatType = $ConnectionParameters.ServiceConfig.ChatType } - # 'IncludeBCCInGroupChat' accepts $null to mean "use the configured value". - if ($PSBoundParameters.ContainsKey('IncludeBCCInGroupChat') -and ($null -ne $IncludeBCCInGroupChat)) + # 'IncludeBCCInGroupChat' accepts $null to mean "use the configured value", which is also what an + # unsupplied [Nullable[bool]] holds. + if ($null -ne $IncludeBCCInGroupChat) { [bool]$ResolvedIncludeBCCInGroupChat = $IncludeBCCInGroupChat } else { - [bool]$ResolvedIncludeBCCInGroupChat = $ConnectionParameters.ServiceConfig.IncludeBCCInGroupChat + [bool]$ResolvedIncludeBCCInGroupChat = Get-ScriptMessageBooleanSetting -Name 'IncludeBCCInGroupChat' ` + -Value $ConnectionParameters.ServiceConfig.IncludeBCCInGroupChat } # Connect to the messaging service, if necessary (e.g., API service). @@ -347,12 +348,6 @@ function Send-ScriptMessage } Send-ScriptMessage_MicrosoftGraph @SendMessageParameters - - # Disconnect from Microsoft Graph API, if enabled in config. - if ($ConnectionParameters.ServiceConfig.MgDisconnectWhenDone) - { - $null = Disconnect-MgGraph -ErrorAction SilentlyContinue - } } Default {throw "Invalid `'Service`' value."} } diff --git a/ScriptMessage/Services/MicrosoftGraph.ps1 b/ScriptMessage/Services/MicrosoftGraph.ps1 index 9bbd3fd..c0e8111 100644 --- a/ScriptMessage/Services/MicrosoftGraph.ps1 +++ b/ScriptMessage/Services/MicrosoftGraph.ps1 @@ -515,6 +515,11 @@ function Send-ScriptMessage_MicrosoftGraph # Get the Service Config. $ServiceConfig = Get-ScriptMessageConfig -Service $ServiceId + # This service's own boolean settings, checked before anything is sent so a value the module cannot read + # stops the send instead of surfacing after the message has already gone out. + $MgDisconnectWhenDone = Get-ScriptMessageBooleanSetting -Name 'MgDisconnectWhenDone' ` + -Value $ServiceConfig.MgDisconnectWhenDone + # Send the message on each supported service specified. foreach ($typeItem in $Type) { @@ -1045,4 +1050,10 @@ function Send-ScriptMessage_MicrosoftGraph } } } + + # Disconnect from the Microsoft Graph API, if enabled in the configuration file. + if ($MgDisconnectWhenDone) + { + $null = Disconnect-MgGraph -ErrorAction SilentlyContinue + } } \ No newline at end of file diff --git a/Tests/Send-ScriptMessage.Tests.ps1 b/Tests/Send-ScriptMessage.Tests.ps1 index 2c0d9ab..b33c5dc 100644 --- a/Tests/Send-ScriptMessage.Tests.ps1 +++ b/Tests/Send-ScriptMessage.Tests.ps1 @@ -483,6 +483,104 @@ Describe 'Send-ScriptMessage configuration defaults' { } } + Context 'Boolean settings and parameters reject strings' { + BeforeAll { + Mock -ModuleName ScriptMessage Disconnect-MgGraph { } + + $MailArguments = @{ + Service = 'MicrosoftGraph' + Type = 'Mail' + From = 'sender@example.org' + To = 'recipient@example.org' + Subject = 'Test subject' + Body = 'Test body' + } + } + + # Parameter binding fails before the configuration file is read, so this needs no configuration mock. + # 'false' is a non-empty string, and every non-empty string casts to $true, so accepting it would mean + # the opposite of what the caller asked for. + It 'rejects a string passed to -IncludeBCCInGroupChat' { + { Send-ScriptMessage @MailArguments -IncludeBCCInGroupChat 'false' } | + Should -Throw '*Boolean parameters accept only*' + } + + Context 'A configuration whose booleans are real booleans' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + IncludeBCCInGroupChat = $true + MgPermissionType = 'Application' + MgDisconnectWhenDone = $true + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'accepts a real $false for -IncludeBCCInGroupChat' { + { Send-ScriptMessage @MailArguments -IncludeBCCInGroupChat $false -ErrorAction Stop } | + Should -Not -Throw + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly + } + + It 'disconnects when MgDisconnectWhenDone is a real $true' { + $null = Send-ScriptMessage @MailArguments + + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly + Should -Invoke -ModuleName ScriptMessage Disconnect-MgGraph -Times 1 -Exactly + } + } + + Context 'A configuration file that quotes IncludeBCCInGroupChat' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + IncludeBCCInGroupChat = 'false' + MgPermissionType = 'Application' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'stops without sending' { + { Send-ScriptMessage @MailArguments } | + Should -Throw "*'IncludeBCCInGroupChat' setting*must be a boolean*" + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 0 -Exactly + } + } + + Context 'A configuration file that quotes MgDisconnectWhenDone' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + IncludeBCCInGroupChat = $false + MgPermissionType = 'Application' + MgDisconnectWhenDone = 'false' + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + # The Microsoft Graph service consumes this setting after its message type loop, so it checks the + # value before that loop to keep an unreadable one from surfacing after the message has gone out. + It 'stops without sending' { + { Send-ScriptMessage @MailArguments } | + Should -Throw "*'MgDisconnectWhenDone' setting*must be a boolean*" + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 0 -Exactly + } + } + } + Context 'A configuration that leaves ChatType blank' { BeforeAll { # Templates/config_scriptmessage.json writes a setting it leaves unset as an empty string, so a From 1cf19cf69f9f1f23f2c3c8eb4a4f5df9b6ec0221 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Thu, 17 Sep 2026 15:36:55 -0500 Subject: [PATCH 04/10] Move Get-MgContext into the Microsoft Graph service - Get-ScriptMessageContext called Get-MgContext directly, so a core cmdlet depended on Microsoft.Graph.Authentication. Add Get-ScriptMessageContext_MicrosoftGraph alongside the existing Disconnect-ScriptMessage_MicrosoftGraph and call that instead. No cmdlet in ScriptMessage/Public/ calls a Microsoft Graph cmdlet now. - Lift the property-merging loop out of the switch arm so it runs on whatever any service returns, which also retires the "break # Terminate the switch statement" that only existed because the emptiness check sat inside the arm. Keep [string]::IsNullOrEmpty for that check so the test is unchanged from what shipped. - Add Pester coverage for this cmdlet, which had none: the service name is returned alongside the service's own properties, a service with no connection yields just the service name, and -ReturnCachedContext does not ask the service twice. One test mocks Get-MgContext rather than the service function, so it covers the whole chain. Output is unchanged, so there is no changelog entry. --- .../Public/Get-ScriptMessageContext.ps1 | 22 ++--- ScriptMessage/Services/MicrosoftGraph.ps1 | 7 ++ Tests/Get-ScriptMessageContext.Tests.ps1 | 81 +++++++++++++++++++ 3 files changed, 99 insertions(+), 11 deletions(-) create mode 100644 Tests/Get-ScriptMessageContext.Tests.ps1 diff --git a/ScriptMessage/Public/Get-ScriptMessageContext.ps1 b/ScriptMessage/Public/Get-ScriptMessageContext.ps1 index 3e15b11..e779201 100644 --- a/ScriptMessage/Public/Get-ScriptMessageContext.ps1 +++ b/ScriptMessage/Public/Get-ScriptMessageContext.ps1 @@ -58,19 +58,19 @@ function Get-ScriptMessageContext $ScriptMessageContext | Add-Member -MemberType NoteProperty -Name "$($infoItem.Name)" -Value $($infoItem.Value) } - # Retrieve connection information. + # Ask the service for its own connection information. switch ($Service) { - MicrosoftGraph { - $MgContext = Get-MgContext - if ([string]::IsNullOrEmpty($MgContext)) - { - break # Terminate the switch statement. - } - foreach ($infoItem in $($MgContext.PSObject.Properties)) - { - $ScriptMessageContext | Add-Member -MemberType NoteProperty -Name "$($infoItem.Name)" -Value $($infoItem.Value) - } + MicrosoftGraph {$ServiceConnectionInfo = Get-ScriptMessageContext_MicrosoftGraph} + } + + # Add in whatever the service returned. A service with no active connection returns nothing, which + # leaves the context object holding only the common information above. + if (-not [string]::IsNullOrEmpty($ServiceConnectionInfo)) + { + foreach ($infoItem in $($ServiceConnectionInfo.PSObject.Properties)) + { + $ScriptMessageContext | Add-Member -MemberType NoteProperty -Name "$($infoItem.Name)" -Value $($infoItem.Value) } } diff --git a/ScriptMessage/Services/MicrosoftGraph.ps1 b/ScriptMessage/Services/MicrosoftGraph.ps1 index c0e8111..ab0af10 100644 --- a/ScriptMessage/Services/MicrosoftGraph.ps1 +++ b/ScriptMessage/Services/MicrosoftGraph.ps1 @@ -422,6 +422,13 @@ function Disconnect-ScriptMessage_MicrosoftGraph return Disconnect-MgGraph } +function Get-ScriptMessageContext_MicrosoftGraph +{ + # Returns nothing when there is no active connection. The caller adds the properties of whatever comes back + # to the context object it returns, so the shape of this is Microsoft Graph's to decide. + return Get-MgContext +} + function Send-ScriptMessage_MicrosoftGraph { [CmdletBinding()] diff --git a/Tests/Get-ScriptMessageContext.Tests.ps1 b/Tests/Get-ScriptMessageContext.Tests.ps1 new file mode 100644 index 0000000..4b1223d --- /dev/null +++ b/Tests/Get-ScriptMessageContext.Tests.ps1 @@ -0,0 +1,81 @@ +# Pester 5 or later. Run from the repository root: +# Invoke-Pester -Path .\Tests +# Nothing here connects to a tenant, and the Microsoft Graph modules do not need to be installed. + +BeforeAll { + Import-Module (Join-Path $PSScriptRoot '..\ScriptMessage\ScriptMessage.psd1') -Force + + # Pester can only mock a command that exists, so define a stand-in for the Microsoft Graph cmdlet inside the + # module before mocking it. It throws if a test reaches it without a mock. + InModuleScope ScriptMessage { + function script:Get-MgContext { throw 'Get-MgContext stand-in called without a mock.' } + } +} + +AfterAll { + Remove-Module ScriptMessage -Force -ErrorAction SilentlyContinue +} + +Describe 'Get-ScriptMessageContext' { + Context 'A service with an active connection' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageContext_MicrosoftGraph { + [pscustomobject]@{ + Account = 'sender@example.org' + Scopes = @('Mail.Send', 'Chat.ReadWrite') + TenantId = 'tenant-id' + } + } + } + + It 'returns the service name alongside the properties the service supplied' { + $Context = Get-ScriptMessageContext -Service MicrosoftGraph + + $Context.Service | Should -Be 'MicrosoftGraph' + $Context.Account | Should -Be 'sender@example.org' + $Context.TenantId | Should -Be 'tenant-id' + $Context.Scopes | Should -Contain 'Chat.ReadWrite' + } + + It 'asks the service rather than calling Microsoft Graph itself' { + $null = Get-ScriptMessageContext -Service MicrosoftGraph + + Should -Invoke -ModuleName ScriptMessage Get-ScriptMessageContext_MicrosoftGraph -Times 1 -Exactly + } + + It 'returns the cached context without asking the service again' { + # The first call has no cache to read, so it refreshes and populates one. + $null = Get-ScriptMessageContext -Service MicrosoftGraph + $Cached = Get-ScriptMessageContext -Service MicrosoftGraph -ReturnCachedContext + + $Cached.Account | Should -Be 'sender@example.org' + Should -Invoke -ModuleName ScriptMessage Get-ScriptMessageContext_MicrosoftGraph -Times 1 -Exactly + } + } + + Context 'A service with no active connection' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageContext_MicrosoftGraph { } + } + + It 'still returns the service name and nothing else' { + $Context = Get-ScriptMessageContext -Service MicrosoftGraph + + $Context.Service | Should -Be 'MicrosoftGraph' + @($Context.PSObject.Properties).Count | Should -Be 1 + } + } + + Context 'The Microsoft Graph service function' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-MgContext { [pscustomobject]@{ Account = 'sender@example.org' } } + } + + It 'is the only place that calls Get-MgContext' { + $Context = Get-ScriptMessageContext -Service MicrosoftGraph + + $Context.Account | Should -Be 'sender@example.org' + Should -Invoke -ModuleName ScriptMessage Get-MgContext -Times 1 -Exactly + } + } +} From 4821cdc57547b25f6763af41ffcd47c38028b0ac Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Thu, 17 Sep 2026 15:55:32 -0500 Subject: [PATCH 05/10] Clear the cached service context when disconnecting - Disconnect-ScriptMessage left ScriptMessage_Global_CachedServiceContext untouched, so Get-ScriptMessageContext -ReturnCachedContext kept returning the account and scopes of a session that had already ended. Remove the service's entry from the cache after the service disconnects, which makes the next cached read refresh instead of replaying it. Removing a name the cache does not hold is a no-op, so calling Disconnect-ScriptMessage before anything has asked for the context is unaffected. - Add Pester coverage for Disconnect-ScriptMessage, which had none: it asks the service to disconnect, returns the service name alongside the service's own disconnection information under -ReturnConnectionInfo, returns nothing without that switch, and clears the cache. Mocking the service functions rather than the Microsoft Graph cmdlets keeps the file free of stand-ins. --- CHANGELOG.md | 1 + .../Public/Disconnect-ScriptMessage.ps1 | 5 ++ Tests/Disconnect-ScriptMessage.Tests.ps1 | 50 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 Tests/Disconnect-ScriptMessage.Tests.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c53b12..e9adfeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - `Send-ScriptMessage` now honors an explicit `-ChatType OneOnOne`. The configuration file's `ChatType` setting was used in its place, so a call asking for one-on-one chats sent a single group chat when that setting was `Group`. An explicit `-ChatType Group` was unaffected. - `Send-ScriptMessage` now honors an explicit `-IncludeBCCInGroupChat $false`. The configuration file's `IncludeBCCInGroupChat` setting was used in its place, so a call asking to keep BCC recipients out of a group chat added them anyway when that setting was `true`, making those addresses visible to everyone in the chat. An explicit `-IncludeBCCInGroupChat $true` was unaffected. - `Send-ScriptMessage` now sends mail without a `ChatType` setting in the configuration file. A Mail-only send failed without it, reporting "Cannot convert null to type ChatType", even though `ChatType` only affects chat; a configuration file listing only `Mail` in `AllowableMessageTypes`, or one written before the setting was added in version 1.0.8, was affected. A `Chat` send with no chat type, because the setting is missing or blank and `-ChatType` was not passed, now reports that in the returned result's `Error` property and names both places it can be set. +- `Get-ScriptMessageContext -ReturnCachedContext` no longer reports a connection that `Disconnect-ScriptMessage` has already ended. Disconnecting left the cached context in place, so a cached read kept returning the account and scopes of the finished session, and only a read without `-ReturnCachedContext` showed the truth. Disconnecting now clears that service's cached context, so the next cached read asks the service again. - Minor: The help for `Get-ScriptMessageConfig` and `Set-ScriptMessageConfigFilePath` now describes the `-Path` parameter, which it left blank. - Minor: The fourth `Send-ScriptMessage` help example, which sends attachments held in variables, now runs as written. It failed with "Missing closing ')' in subexpression" when copied, and it ended without calling `Send-ScriptMessage`. diff --git a/ScriptMessage/Public/Disconnect-ScriptMessage.ps1 b/ScriptMessage/Public/Disconnect-ScriptMessage.ps1 index 4f29b3d..8163822 100644 --- a/ScriptMessage/Public/Disconnect-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Disconnect-ScriptMessage.ps1 @@ -43,6 +43,11 @@ function Disconnect-ScriptMessage MicrosoftGraph {Disconnect-ScriptMessage_MicrosoftGraph} } + # Drop this service's cached context. 'Get-ScriptMessageContext -ReturnCachedContext' reads that cache, so + # leaving an entry behind would keep reporting the connection this call just ended. Removing a name the + # cache does not hold does nothing, which is the case when nothing has asked for the context yet. + $ScriptMessage_Global_CachedServiceContext.PSObject.Properties.Remove([string]$Service) + # Return the disconnection information, if requested. if ($ReturnConnectionInfo) { diff --git a/Tests/Disconnect-ScriptMessage.Tests.ps1 b/Tests/Disconnect-ScriptMessage.Tests.ps1 new file mode 100644 index 0000000..9091fb2 --- /dev/null +++ b/Tests/Disconnect-ScriptMessage.Tests.ps1 @@ -0,0 +1,50 @@ +# Pester 5 or later. Run from the repository root: +# Invoke-Pester -Path .\Tests +# Nothing here connects to a tenant, and the Microsoft Graph modules do not need to be installed. Mocking the +# service functions rather than the Microsoft Graph cmdlets they call keeps that true without stand-ins. + +BeforeAll { + Import-Module (Join-Path $PSScriptRoot '..\ScriptMessage\ScriptMessage.psd1') -Force +} + +AfterAll { + Remove-Module ScriptMessage -Force -ErrorAction SilentlyContinue +} + +Describe 'Disconnect-ScriptMessage' { + BeforeAll { + Mock -ModuleName ScriptMessage Disconnect-ScriptMessage_MicrosoftGraph { + [pscustomobject]@{ Account = 'sender@example.org' } + } + Mock -ModuleName ScriptMessage Get-ScriptMessageContext_MicrosoftGraph { + [pscustomobject]@{ Account = 'sender@example.org' } + } + } + + It 'asks the service to disconnect' { + $null = Disconnect-ScriptMessage -Service MicrosoftGraph + + Should -Invoke -ModuleName ScriptMessage Disconnect-ScriptMessage_MicrosoftGraph -Times 1 -Exactly + } + + It 'returns the service name with the disconnection information when asked' { + $Info = Disconnect-ScriptMessage -Service MicrosoftGraph -ReturnConnectionInfo + + $Info.Service | Should -Be 'MicrosoftGraph' + $Info.Account | Should -Be 'sender@example.org' + } + + It 'returns nothing unless -ReturnConnectionInfo is given' { + Disconnect-ScriptMessage -Service MicrosoftGraph | Should -BeNullOrEmpty + } + + It 'clears the cached context, so -ReturnCachedContext stops reporting the old connection' { + # Populate the cache, disconnect, then read it back. A cleared cache forces the read to ask the service + # again, which is the only way a caller can tell that the cached connection is gone. + $null = Get-ScriptMessageContext -Service MicrosoftGraph + $null = Disconnect-ScriptMessage -Service MicrosoftGraph + $null = Get-ScriptMessageContext -Service MicrosoftGraph -ReturnCachedContext + + Should -Invoke -ModuleName ScriptMessage Get-ScriptMessageContext_MicrosoftGraph -Times 2 -Exactly + } +} From 0f8f7d3041ee59dfb923eef9cfc2cc7bd2628d35 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Thu, 17 Sep 2026 16:10:40 -0500 Subject: [PATCH 06/10] Dispatch to messaging services through a service table - Each public cmdlet named MicrosoftGraph in a switch block, five in all, so adding a service meant editing four files in the core. Add a module-scope service table that each file in Services/ registers itself into as it is dot-sourced, and replace every switch with a lookup plus a call. - Record handlers as function NAMES and resolve them with & at call time. Caching a CommandInfo or a script block at registration would bind to the real function and bypass Pester, which mocks by shadowing a command name. - Register-ScriptMessageService rejects a name that is not a MessagingService member, since every -Service parameter is typed to that enum and a service outside it could never be reached. Get-ScriptMessageServiceRegistration throws naming the registered services, replacing a Default arm that said only "Invalid 'Service' value." - Disconnect-ScriptMessage loses its second switch: the loop that merges the service's return information is service-agnostic, so it now runs on whatever comes back, which retires another "break # Terminate the switch statement". - Add Pester coverage: the table holds the Graph handlers, an unknown service names the registered ones, a non-enum name is refused, and all four public cmdlets reach stand-in handlers swapped into the table with no Microsoft Graph mocks loaded, which is what proves the core reaches a service only through the table. Output is unchanged, so there is no changelog entry. --- .../Get-ScriptMessageServiceRegistration.ps1 | 23 ++++ .../Private/Register-ScriptMessageService.ps1 | 41 ++++++ .../Public/Connect-ScriptMessage.ps1 | 6 +- .../Public/Disconnect-ScriptMessage.ps1 | 23 ++-- .../Public/Get-ScriptMessageContext.ps1 | 6 +- ScriptMessage/Public/Send-ScriptMessage.ps1 | 53 ++++---- ScriptMessage/ScriptMessage.psm1 | 5 + ScriptMessage/Services/MicrosoftGraph.ps1 | 9 +- Tests/ServiceDispatch.Tests.ps1 | 119 ++++++++++++++++++ 9 files changed, 233 insertions(+), 52 deletions(-) create mode 100644 ScriptMessage/Private/Get-ScriptMessageServiceRegistration.ps1 create mode 100644 ScriptMessage/Private/Register-ScriptMessageService.ps1 create mode 100644 Tests/ServiceDispatch.Tests.ps1 diff --git a/ScriptMessage/Private/Get-ScriptMessageServiceRegistration.ps1 b/ScriptMessage/Private/Get-ScriptMessageServiceRegistration.ps1 new file mode 100644 index 0000000..b251a40 --- /dev/null +++ b/ScriptMessage/Private/Get-ScriptMessageServiceRegistration.ps1 @@ -0,0 +1,23 @@ +function Get-ScriptMessageServiceRegistration +{ + <# + Returns the service table entry a public cmdlet needs in order to reach a service's handlers. Takes the + service as a string so that no caller has to hold the [MessagingService] type. + #> + + [CmdletBinding()] + param( + [Parameter(Mandatory = $true)] + [string]$Service + ) + + $Registration = $script:ScriptMessageServiceTable[$Service] + + if ($null -eq $Registration) + { + $Registered = $script:ScriptMessageServiceTable.Keys -join ', ' + throw "`'$Service`' is not a registered messaging service. Registered services: $Registered." + } + + return $Registration +} diff --git a/ScriptMessage/Private/Register-ScriptMessageService.ps1 b/ScriptMessage/Private/Register-ScriptMessageService.ps1 new file mode 100644 index 0000000..ca41bc3 --- /dev/null +++ b/ScriptMessage/Private/Register-ScriptMessageService.ps1 @@ -0,0 +1,41 @@ +function Register-ScriptMessageService +{ + <# + Called once by each file in Services/ as it is dot-sourced, so the public cmdlets can dispatch to a + service without naming it. Handlers are recorded as function NAMES, not as command objects or script + blocks: the name is resolved at call time, which is what lets Pester's mocks shadow a handler. + #> + + [CmdletBinding()] + param( + [Parameter(Mandatory = $true)] + [string]$Name, + + [Parameter(Mandatory = $true)] + [string]$ConnectFunction, + + [Parameter(Mandatory = $true)] + [string]$DisconnectFunction, + + [Parameter(Mandatory = $true)] + [string]$SendFunction, + + [Parameter(Mandatory = $true)] + [string]$GetContextFunction + ) + + # Every public cmdlet types its '-Service' parameter as [MessagingService], so a service registered under a + # name outside that enum could never be reached. Fail at import rather than leaving it silently unusable. + if ($Name -notin [Enum]::GetNames([MessagingService])) + { + throw "Cannot register the messaging service `'$Name`': it is not a member of the MessagingService enum. Add it to the enum in ScriptMessage.psm1 first." + } + + $script:ScriptMessageServiceTable[$Name] = [PSCustomObject]@{ + Name = $Name + ConnectFunction = $ConnectFunction + DisconnectFunction = $DisconnectFunction + SendFunction = $SendFunction + GetContextFunction = $GetContextFunction + } +} diff --git a/ScriptMessage/Public/Connect-ScriptMessage.ps1 b/ScriptMessage/Public/Connect-ScriptMessage.ps1 index 746dcd8..59786cd 100644 --- a/ScriptMessage/Public/Connect-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Connect-ScriptMessage.ps1 @@ -43,10 +43,8 @@ function Connect-ScriptMessage } # Connect to the proper service. Each service checks for the modules its allowed message types need. - switch ($Service) - { - MicrosoftGraph {Connect-ScriptMessage_MicrosoftGraph @ConnectionParameters} - } + $Registration = Get-ScriptMessageServiceRegistration -Service $Service + & $Registration.ConnectFunction @ConnectionParameters # Return the connection information, if requested. if ($ReturnConnectionInfo) diff --git a/ScriptMessage/Public/Disconnect-ScriptMessage.ps1 b/ScriptMessage/Public/Disconnect-ScriptMessage.ps1 index 8163822..5d5afc5 100644 --- a/ScriptMessage/Public/Disconnect-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Disconnect-ScriptMessage.ps1 @@ -38,10 +38,8 @@ function Disconnect-ScriptMessage ) # Disconnect from the proper service. - $ServiceDisconnectReturnInfo = switch ($Service) - { - MicrosoftGraph {Disconnect-ScriptMessage_MicrosoftGraph} - } + $Registration = Get-ScriptMessageServiceRegistration -Service $Service + $ServiceDisconnectReturnInfo = & $Registration.DisconnectFunction # Drop this service's cached context. 'Get-ScriptMessageContext -ReturnCachedContext' reads that cache, so # leaving an entry behind would keep reporting the connection this call just ended. Removing a name the @@ -63,18 +61,13 @@ function Disconnect-ScriptMessage $ScriptMessageDisconnectReturnInfo | Add-Member -MemberType NoteProperty -Name "$($infoItem.Name)" -Value $($infoItem.Value) } - # Add in disconnection information. - switch ($Service) + # Add in whatever the service returned. A service that reports nothing back leaves the object holding + # only the common information above. + if (-not [string]::IsNullOrEmpty($ServiceDisconnectReturnInfo)) { - MicrosoftGraph { - if ([string]::IsNullOrEmpty($ServiceDisconnectReturnInfo)) - { - break # Terminate the switch statement. - } - foreach ($infoItem in $($ServiceDisconnectReturnInfo.PSObject.Properties)) - { - $ScriptMessageDisconnectReturnInfo | Add-Member -MemberType NoteProperty -Name "$($infoItem.Name)" -Value $($infoItem.Value) - } + foreach ($infoItem in $($ServiceDisconnectReturnInfo.PSObject.Properties)) + { + $ScriptMessageDisconnectReturnInfo | Add-Member -MemberType NoteProperty -Name "$($infoItem.Name)" -Value $($infoItem.Value) } } } diff --git a/ScriptMessage/Public/Get-ScriptMessageContext.ps1 b/ScriptMessage/Public/Get-ScriptMessageContext.ps1 index e779201..9a4d964 100644 --- a/ScriptMessage/Public/Get-ScriptMessageContext.ps1 +++ b/ScriptMessage/Public/Get-ScriptMessageContext.ps1 @@ -59,10 +59,8 @@ function Get-ScriptMessageContext } # Ask the service for its own connection information. - switch ($Service) - { - MicrosoftGraph {$ServiceConnectionInfo = Get-ScriptMessageContext_MicrosoftGraph} - } + $Registration = Get-ScriptMessageServiceRegistration -Service $Service + $ServiceConnectionInfo = & $Registration.GetContextFunction # Add in whatever the service returned. A service with no active connection returns nothing, which # leaves the context object holding only the common information above. diff --git a/ScriptMessage/Public/Send-ScriptMessage.ps1 b/ScriptMessage/Public/Send-ScriptMessage.ps1 index 7ee0bc2..be76fea 100644 --- a/ScriptMessage/Public/Send-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Send-ScriptMessage.ps1 @@ -287,6 +287,9 @@ function Send-ScriptMessage foreach ($serviceTypeObj in $ServiceType) # TODO: Catch errors once we have multiple services so if one fails the other(s) can still be processed. { + # Look the service up first, so an unregistered one stops the call before any other work. + $Registration = Get-ScriptMessageServiceRegistration -Service ([string]$serviceTypeObj.Service) + # Set the connection parameters. $ConnectionParameters = @{ ServiceConfig = $ScriptMessageConfig.$($serviceTypeObj.Service) @@ -321,35 +324,29 @@ function Send-ScriptMessage # Connect to the messaging service, if necessary (e.g., API service). Connect-ScriptMessage -Service $($serviceTypeObj.Service) -ErrorAction Stop - switch ($($serviceTypeObj.Service)) + $SendMessageParameters = [ordered]@{ + From = $From + ReplyTo = $ReplyTo + To = $To + CC = $CC + BCC = $BCC + SaveToSentItems = $SaveToSentItems + Subject = $Subject + Body = $Body + Attachment = $Attachment + SenderId = $SenderId + Type = $serviceTypeObj.Type + IncludeBCCInGroupChat = $ResolvedIncludeBCCInGroupChat + } + + # [ChatType] cannot be bound to $null, nor to the empty string a configuration file uses for a setting + # it leaves unset, so pass it only when there is a real value. A Mail-only send does not need one, and + # the service reports a Chat message that has none. + if (-not [string]::IsNullOrWhiteSpace($ResolvedChatType)) { - 'MicrosoftGraph' { - $SendMessageParameters = [ordered]@{ - From = $From - ReplyTo = $ReplyTo - To = $To - CC = $CC - BCC = $BCC - SaveToSentItems = $SaveToSentItems - Subject = $Subject - Body = $Body - Attachment = $Attachment - SenderId = $SenderId - Type = $serviceTypeObj.Type - IncludeBCCInGroupChat = $ResolvedIncludeBCCInGroupChat - } - - # [ChatType] cannot be bound to $null, nor to the empty string a configuration file uses for a - # setting it leaves unset, so pass it only when there is a real value. A Mail-only send does not - # need one, and the service reports a Chat message that has none. - if (-not [string]::IsNullOrWhiteSpace($ResolvedChatType)) - { - $SendMessageParameters['ChatType'] = $ResolvedChatType - } - - Send-ScriptMessage_MicrosoftGraph @SendMessageParameters - } - Default {throw "Invalid `'Service`' value."} + $SendMessageParameters['ChatType'] = $ResolvedChatType } + + & $Registration.SendFunction @SendMessageParameters } } \ No newline at end of file diff --git a/ScriptMessage/ScriptMessage.psm1 b/ScriptMessage/ScriptMessage.psm1 index e8998a0..e0216f7 100644 --- a/ScriptMessage/ScriptMessage.psm1 +++ b/ScriptMessage/ScriptMessage.psm1 @@ -1,6 +1,11 @@ # Global Variables New-Variable -Name 'ScriptMessage_Global_CachedServiceContext' -Value ([PSCustomObject]@{}) -Scope Global -Force +# Module Variables +# Each file in Services/ registers itself here as it is dot-sourced below. Module scope rather than global: +# nothing outside the module reads it, and it is rebuilt from scratch on every import. +$script:ScriptMessageServiceTable = [ordered]@{} + # Aliases # Type Definitions diff --git a/ScriptMessage/Services/MicrosoftGraph.ps1 b/ScriptMessage/Services/MicrosoftGraph.ps1 index ab0af10..4a13302 100644 --- a/ScriptMessage/Services/MicrosoftGraph.ps1 +++ b/ScriptMessage/Services/MicrosoftGraph.ps1 @@ -1063,4 +1063,11 @@ function Send-ScriptMessage_MicrosoftGraph { $null = Disconnect-MgGraph -ErrorAction SilentlyContinue } -} \ No newline at end of file +} + +# Announce this service to the module. Keep this at the bottom of the file, after the functions it names. +Register-ScriptMessageService -Name 'MicrosoftGraph' ` + -ConnectFunction 'Connect-ScriptMessage_MicrosoftGraph' ` + -DisconnectFunction 'Disconnect-ScriptMessage_MicrosoftGraph' ` + -SendFunction 'Send-ScriptMessage_MicrosoftGraph' ` + -GetContextFunction 'Get-ScriptMessageContext_MicrosoftGraph' \ No newline at end of file diff --git a/Tests/ServiceDispatch.Tests.ps1 b/Tests/ServiceDispatch.Tests.ps1 new file mode 100644 index 0000000..b45cda5 --- /dev/null +++ b/Tests/ServiceDispatch.Tests.ps1 @@ -0,0 +1,119 @@ +# Pester 5 or later. Run from the repository root: +# Invoke-Pester -Path .\Tests +# Nothing here connects to a tenant, and the Microsoft Graph modules do not need to be installed. + +BeforeAll { + Import-Module (Join-Path $PSScriptRoot '..\ScriptMessage\ScriptMessage.psd1') -Force +} + +AfterAll { + Remove-Module ScriptMessage -Force -ErrorAction SilentlyContinue +} + +Describe 'The service table' { + It 'holds the Microsoft Graph service with all four handlers' { + InModuleScope ScriptMessage { + $Registration = Get-ScriptMessageServiceRegistration -Service 'MicrosoftGraph' + + $Registration.ConnectFunction | Should -Be 'Connect-ScriptMessage_MicrosoftGraph' + $Registration.DisconnectFunction | Should -Be 'Disconnect-ScriptMessage_MicrosoftGraph' + $Registration.SendFunction | Should -Be 'Send-ScriptMessage_MicrosoftGraph' + $Registration.GetContextFunction | Should -Be 'Get-ScriptMessageContext_MicrosoftGraph' + } + } + + It 'names the registered services when asked for one it does not have' { + InModuleScope ScriptMessage { + { Get-ScriptMessageServiceRegistration -Service 'NotAService' } | + Should -Throw "*not a registered messaging service*MicrosoftGraph*" + } + } + + It 'refuses to register a service that is not a MessagingService member' { + InModuleScope ScriptMessage { + { Register-ScriptMessageService -Name 'Carrier Pigeon' ` + -ConnectFunction 'c' -DisconnectFunction 'd' -SendFunction 's' -GetContextFunction 'g' } | + Should -Throw '*not a member of the MessagingService enum*' + } + } +} + +Describe 'The public cmdlets dispatch through the service table' { + # Re-point the Microsoft Graph entry at stand-in handlers and drive the public cmdlets with no Microsoft + # Graph mocks loaded at all. If they still work, the core reaches a service only through the table. A fake + # service cannot be registered under a new name while -Service is the MessagingService enum, which is why + # this replaces an existing entry rather than adding one. + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + IncludeBCCInGroupChat = $false + MgPermissionType = 'Application' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + + InModuleScope ScriptMessage { + function script:Connect-StandInService { param($ServiceConfig) $script:StandInCalls += 'Connect' } + function script:Disconnect-StandInService { $script:StandInCalls += 'Disconnect'; [pscustomobject]@{ StandIn = $true } } + function script:Get-StandInServiceContext { $script:StandInCalls += 'GetContext'; [pscustomobject]@{ StandIn = $true } } + function script:Send-StandInService { $script:StandInCalls += 'Send'; [pscustomobject]@{ StandIn = $true } } + + $script:OriginalRegistration = $script:ScriptMessageServiceTable['MicrosoftGraph'] + $script:ScriptMessageServiceTable['MicrosoftGraph'] = [PSCustomObject]@{ + Name = 'MicrosoftGraph' + ConnectFunction = 'Connect-StandInService' + DisconnectFunction = 'Disconnect-StandInService' + SendFunction = 'Send-StandInService' + GetContextFunction = 'Get-StandInServiceContext' + } + } + } + + AfterAll { + # Leaving a stand-in in the table would break every later test file that expects the real service. + InModuleScope ScriptMessage { + $script:ScriptMessageServiceTable['MicrosoftGraph'] = $script:OriginalRegistration + } + } + + BeforeEach { + InModuleScope ScriptMessage { $script:StandInCalls = @() } + } + + It 'Connect-ScriptMessage reaches the registered connect handler' { + $null = Connect-ScriptMessage -Service MicrosoftGraph + + InModuleScope ScriptMessage { $script:StandInCalls | Should -Contain 'Connect' } + } + + It 'Disconnect-ScriptMessage reaches the registered disconnect handler' { + $Info = Disconnect-ScriptMessage -Service MicrosoftGraph -ReturnConnectionInfo + + $Info.Service | Should -Be 'MicrosoftGraph' + $Info.StandIn | Should -BeTrue + InModuleScope ScriptMessage { $script:StandInCalls | Should -Contain 'Disconnect' } + } + + It 'Get-ScriptMessageContext reaches the registered context handler' { + $Context = Get-ScriptMessageContext -Service MicrosoftGraph + + $Context.Service | Should -Be 'MicrosoftGraph' + $Context.StandIn | Should -BeTrue + InModuleScope ScriptMessage { $script:StandInCalls | Should -Contain 'GetContext' } + } + + It 'Send-ScriptMessage reaches the registered send handler, connecting on the way' { + $Result = Send-ScriptMessage -Service MicrosoftGraph -Type Mail -From 'sender@example.org' ` + -To 'recipient@example.org' -Subject 'Test subject' -Body 'Test body' + + $Result.StandIn | Should -BeTrue + InModuleScope ScriptMessage { + $script:StandInCalls | Should -Contain 'Connect' + $script:StandInCalls | Should -Contain 'Send' + } + } +} From 5e2b5a27b8054823a0cd2322a7a2bfe77cdb13c1 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Thu, 17 Sep 2026 16:20:03 -0500 Subject: [PATCH 07/10] Read the configuration file once per send - Send-ScriptMessage, Connect-ScriptMessage and Send-ScriptMessage_MicrosoftGraph each read the file, so one send read it three times, and five times for two services. Each read reopens the file, redoes the encoding fallback and reparses the JSON, so a send had no single consistent view of its own settings: editing the file mid-send could hand the connection different credentials than the checks that followed it. - Send-ScriptMessage now reads once and passes the service's section to Connect-ScriptMessage and to the service through a new optional -ServiceConfig parameter. Both still read the file when nobody passes it, so calling either on its own is unchanged. - Resolve the section with the registered service name rather than the -Service parameter's value, because looking a property up by a name held in an array comes back empty. A configuration file with no section for the service now says so instead of carrying $null into the connection. - Add Pester coverage asserting one read across a Mail and Chat send, and that Connect-ScriptMessage on its own still reads the file. No changelog entry: passing a configuration section was already possible through the service functions, and -ServiceConfig is plumbing rather than a feature. The wiki will cover it. --- .../Public/Connect-ScriptMessage.ps1 | 21 +++++++++-- ScriptMessage/Public/Send-ScriptMessage.ps1 | 13 ++++++- ScriptMessage/Services/MicrosoftGraph.ps1 | 16 ++++++-- Tests/Send-ScriptMessage.Tests.ps1 | 37 +++++++++++++++++++ 4 files changed, 77 insertions(+), 10 deletions(-) diff --git a/ScriptMessage/Public/Connect-ScriptMessage.ps1 b/ScriptMessage/Public/Connect-ScriptMessage.ps1 index 59786cd..1da54f8 100644 --- a/ScriptMessage/Public/Connect-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Connect-ScriptMessage.ps1 @@ -12,6 +12,9 @@ function Connect-ScriptMessage .PARAMETER Service Specify the messaging service to connect to. + .PARAMETER ServiceConfig + Optional. The messaging service's section of the configuration file, for a caller that has already read it. + If not provided, the configuration file is read. .PARAMETER ReturnConnectionInfo Returns connection information after performing function. @@ -29,6 +32,11 @@ function Connect-ScriptMessage ValueFromPipelineByPropertyName = $true)] [MessagingService]$Service, + [Parameter( + Mandatory = $false, + ValueFromPipelineByPropertyName = $true)] + [pscustomobject]$ServiceConfig, + [parameter( Position=1, Mandatory=$false, @@ -36,12 +44,17 @@ function Connect-ScriptMessage ValueFromPipelineByPropertyName=$true)] [switch]$ReturnConnectionInfo ) - - # Set the connection parameters. + + # Set the connection parameters. A caller that has already read the configuration file passes the service's + # section, which is what keeps one send to a single read of the file. + if (-not $PSBoundParameters.ContainsKey('ServiceConfig')) + { + $ServiceConfig = Get-ScriptMessageConfig -Service $Service + } $ConnectionParameters = @{ - ServiceConfig = Get-ScriptMessageConfig -Service $Service + ServiceConfig = $ServiceConfig } - + # Connect to the proper service. Each service checks for the modules its allowed message types need. $Registration = Get-ScriptMessageServiceRegistration -Service $Service & $Registration.ConnectFunction @ConnectionParameters diff --git a/ScriptMessage/Public/Send-ScriptMessage.ps1 b/ScriptMessage/Public/Send-ScriptMessage.ps1 index be76fea..ed88f86 100644 --- a/ScriptMessage/Public/Send-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Send-ScriptMessage.ps1 @@ -290,9 +290,17 @@ function Send-ScriptMessage # Look the service up first, so an unregistered one stops the call before any other work. $Registration = Get-ScriptMessageServiceRegistration -Service ([string]$serviceTypeObj.Service) + # Take this service's section out of the configuration read above, using the registered name rather + # than the parameter's value: looking a property up by a name held in an array comes back empty. + $ServiceConfig = $ScriptMessageConfig.$($Registration.Name) + if ($null -eq $ServiceConfig) + { + throw "The ScriptMessage configuration file has no `'$($Registration.Name)`' section. Add one for that messaging service, or send from a service the file configures." + } + # Set the connection parameters. $ConnectionParameters = @{ - ServiceConfig = $ScriptMessageConfig.$($serviceTypeObj.Service) + ServiceConfig = $ServiceConfig } # Set default values for anything the caller did not specify. These test the bound parameters rather than @@ -322,7 +330,7 @@ function Send-ScriptMessage } # Connect to the messaging service, if necessary (e.g., API service). - Connect-ScriptMessage -Service $($serviceTypeObj.Service) -ErrorAction Stop + Connect-ScriptMessage -Service $($serviceTypeObj.Service) -ServiceConfig $ServiceConfig -ErrorAction Stop $SendMessageParameters = [ordered]@{ From = $From @@ -337,6 +345,7 @@ function Send-ScriptMessage SenderId = $SenderId Type = $serviceTypeObj.Type IncludeBCCInGroupChat = $ResolvedIncludeBCCInGroupChat + ServiceConfig = $ServiceConfig } # [ChatType] cannot be bound to $null, nor to the empty string a configuration file uses for a setting diff --git a/ScriptMessage/Services/MicrosoftGraph.ps1 b/ScriptMessage/Services/MicrosoftGraph.ps1 index 4a13302..729640d 100644 --- a/ScriptMessage/Services/MicrosoftGraph.ps1 +++ b/ScriptMessage/Services/MicrosoftGraph.ps1 @@ -512,15 +512,23 @@ function Send-ScriptMessage_MicrosoftGraph Mandatory = $false, ValueFromPipeline = $true, ValueFromPipelineByPropertyName = $true)] - [bool]$IncludeBCCInGroupChat + [bool]$IncludeBCCInGroupChat, + + [Parameter( + Mandatory = $false, + ValueFromPipelineByPropertyName = $true)] + [pscustomobject]$ServiceConfig ) # Set the Service ID. - # Keep this as a STRING and not the ENUM type since it's returned to the caller (functions will convert to [MessagingService] type as needed). + # Keep this as a STRING and not the ENUM type since it's returned to the caller (functions will convert to [MessagingService] type as needed). [string]$ServiceId = 'MicrosoftGraph' - # Get the Service Config. - $ServiceConfig = Get-ScriptMessageConfig -Service $ServiceId + # Get the Service Config, unless the caller already read it and passed this service's section. + if (-not $PSBoundParameters.ContainsKey('ServiceConfig')) + { + $ServiceConfig = Get-ScriptMessageConfig -Service $ServiceId + } # This service's own boolean settings, checked before anything is sent so a value the module cannot read # stops the send instead of surfacing after the message has already gone out. diff --git a/Tests/Send-ScriptMessage.Tests.ps1 b/Tests/Send-ScriptMessage.Tests.ps1 index b33c5dc..071a79d 100644 --- a/Tests/Send-ScriptMessage.Tests.ps1 +++ b/Tests/Send-ScriptMessage.Tests.ps1 @@ -581,6 +581,43 @@ Describe 'Send-ScriptMessage configuration defaults' { } } + Context 'The configuration file is read once per send' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail', 'Chat') + ChatType = 'OneOnOne' + IncludeBCCInGroupChat = $false + MgPermissionType = 'Delegated' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + # Send-ScriptMessage reads the file, then hands the service's section to Connect-ScriptMessage and to + # the service, so neither reads it again. Three reads gave a send no single consistent view of its own + # settings, because the file can change between them. + It 'reads the configuration file once for a Mail and Chat send' { + $Arguments = $ChatArguments.Clone() + $Arguments['Type'] = @('Mail', 'Chat') + + $null = Send-ScriptMessage @Arguments -WarningAction SilentlyContinue + + Should -Invoke -ModuleName ScriptMessage Get-ScriptMessageConfig -Times 1 -Exactly + } + + It 'still reads the file when Connect-ScriptMessage is called on its own' { + Mock -ModuleName ScriptMessage Connect-ScriptMessage_MicrosoftGraph { } + + $null = Connect-ScriptMessage -Service MicrosoftGraph + + Should -Invoke -ModuleName ScriptMessage Get-ScriptMessageConfig -Times 1 -Exactly + } + } + Context 'A configuration that leaves ChatType blank' { BeforeAll { # Templates/config_scriptmessage.json writes a setting it leaves unset as an empty string, so a From 07615b25b3b7461e81c19cbb1e8f65b8de4de2b8 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Thu, 17 Sep 2026 16:40:09 -0500 Subject: [PATCH 08/10] Declare which generic settings each messaging service supports - -MailType was accepted and then ignored: the core never forwarded it, no service acts on it, and the Mail result reads an undefined variable into its MailType property. Services are meant to be interchangeable, so a setting a service cannot act on is an ordinary case, but dropping it without a word is not. - Register-ScriptMessageService now requires -SupportedSetting, the generic unprefixed settings a service honors. Mandatory so that adding a service forces an answer. Microsoft Graph declares ChatType and IncludeBCCInGroupChat, and not MailType. - Send-ScriptMessage warns when a caller supplies a generic setting the service does not declare, then sends anyway. A warning rather than an error because a single call can go to several services and only some may honor it. A $null is skipped, since that means "use the configured value". - The generic defaults stay in the core rather than moving into the service, as the original plan had it. ChatType and IncludeBCCInGroupChat are generic vocabulary, so resolving them per service would duplicate the same merge rule in every service. - Fix the .PARAMETER MailType help directive, whose trailing "# TODO:" comment stopped Get-Help matching it to the parameter, so Get-Help fell back to the code comment above the declaration and showed an internal note. --- CHANGELOG.md | 2 + .../Private/Register-ScriptMessageService.ps1 | 9 +++ ScriptMessage/Public/Send-ScriptMessage.ps1 | 24 +++++++- ScriptMessage/Services/MicrosoftGraph.ps1 | 2 + Tests/ServiceDispatch.Tests.ps1 | 60 ++++++++++++++++++- 5 files changed, 95 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9adfeb..acce9a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,9 @@ - `Send-ScriptMessage` now honors an explicit `-IncludeBCCInGroupChat $false`. The configuration file's `IncludeBCCInGroupChat` setting was used in its place, so a call asking to keep BCC recipients out of a group chat added them anyway when that setting was `true`, making those addresses visible to everyone in the chat. An explicit `-IncludeBCCInGroupChat $true` was unaffected. - `Send-ScriptMessage` now sends mail without a `ChatType` setting in the configuration file. A Mail-only send failed without it, reporting "Cannot convert null to type ChatType", even though `ChatType` only affects chat; a configuration file listing only `Mail` in `AllowableMessageTypes`, or one written before the setting was added in version 1.0.8, was affected. A `Chat` send with no chat type, because the setting is missing or blank and `-ChatType` was not passed, now reports that in the returned result's `Error` property and names both places it can be set. - `Get-ScriptMessageContext -ReturnCachedContext` no longer reports a connection that `Disconnect-ScriptMessage` has already ended. Disconnecting left the cached context in place, so a cached read kept returning the account and scopes of the finished session, and only a read without `-ReturnCachedContext` showed the truth. Disconnecting now clears that service's cached context, so the next cached read asks the service again. +- `Send-ScriptMessage` now reports that `-MailType` was ignored instead of accepting it in silence. No messaging service acts on that parameter, so a call passing it behaved as though it had not, and the returned result's `MailType` property is empty for the same reason. The value is still ignored, but the call now warns which service does not support it and carries on sending. - Minor: The help for `Get-ScriptMessageConfig` and `Set-ScriptMessageConfigFilePath` now describes the `-Path` parameter, which it left blank. +- Minor: `Get-Help Send-ScriptMessage -Parameter MailType` now describes the parameter. It returned an internal note reading "TODO: Implement MailType (similar to ChatType so we can do 1:1 emailing)". - Minor: The fourth `Send-ScriptMessage` help example, which sends attachments held in variables, now runs as written. It failed with "Missing closing ')' in subexpression" when copied, and it ended without calling `Send-ScriptMessage`. --- diff --git a/ScriptMessage/Private/Register-ScriptMessageService.ps1 b/ScriptMessage/Private/Register-ScriptMessageService.ps1 index ca41bc3..b35dfc5 100644 --- a/ScriptMessage/Private/Register-ScriptMessageService.ps1 +++ b/ScriptMessage/Private/Register-ScriptMessageService.ps1 @@ -11,6 +11,14 @@ function Register-ScriptMessageService [Parameter(Mandatory = $true)] [string]$Name, + # The generic, unprefixed configuration settings and parameters this service honors. Mandatory so that + # every service has to state its answer: a caller who supplies one a service does not honor is told, + # rather than having the value dropped without a word. A service's own prefixed settings are its + # business and are not listed here. + [Parameter(Mandatory = $true)] + [AllowEmptyCollection()] + [string[]]$SupportedSetting, + [Parameter(Mandatory = $true)] [string]$ConnectFunction, @@ -33,6 +41,7 @@ function Register-ScriptMessageService $script:ScriptMessageServiceTable[$Name] = [PSCustomObject]@{ Name = $Name + SupportedSetting = $SupportedSetting ConnectFunction = $ConnectFunction DisconnectFunction = $DisconnectFunction SendFunction = $SendFunction diff --git a/ScriptMessage/Public/Send-ScriptMessage.ps1 b/ScriptMessage/Public/Send-ScriptMessage.ps1 index ed88f86..5794b78 100644 --- a/ScriptMessage/Public/Send-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Send-ScriptMessage.ps1 @@ -49,8 +49,9 @@ function Send-ScriptMessage .PARAMETER SenderId Specify the account used to send the message request. This might be different than the 'From' parameter in the case of "Send As', "Send on Behalf", delegated mailboxes, etc. If not specified, defaults to the address inside of the 'From' parameter. - .PARAMETER MailType # TODO: Implement OneOnOne emailing. + .PARAMETER MailType Override the default 'MailType' specified in the configuration file for the messaging service being used. Options are 'OneOnOne' or 'Group'. + No messaging service supports this parameter yet, so a value given here is reported as unsupported and ignored. .PARAMETER ChatType Override the default 'ChatType' specified in the configuration file for the messaging service being used. Options are 'OneOnOne' or 'Group'. .PARAMETER IncludeBCCInGroupChat @@ -298,6 +299,27 @@ function Send-ScriptMessage throw "The ScriptMessage configuration file has no `'$($Registration.Name)`' section. Add one for that messaging service, or send from a service the file configures." } + # Tell the caller about any generic setting this service does not honor. Services are meant to be + # interchangeable, so one that cannot act on a setting is an ordinary case rather than an error: the + # rest of the send goes ahead, which matters most when a single call goes to several services and only + # some of them honor it. + foreach ($settingName in @('MailType', 'ChatType', 'IncludeBCCInGroupChat')) + { + if (-not $PSBoundParameters.ContainsKey($settingName)) + { + continue + } + # A $null means "use the configured value", so the caller has expressed no preference to drop. + if ($null -eq $PSBoundParameters[$settingName]) + { + continue + } + if ($Registration.SupportedSetting -notcontains $settingName) + { + Write-Warning -Message "The `'$($Registration.Name)`' messaging service does not support the `'$settingName`' parameter, so it was ignored." + } + } + # Set the connection parameters. $ConnectionParameters = @{ ServiceConfig = $ServiceConfig diff --git a/ScriptMessage/Services/MicrosoftGraph.ps1 b/ScriptMessage/Services/MicrosoftGraph.ps1 index 729640d..9bcff88 100644 --- a/ScriptMessage/Services/MicrosoftGraph.ps1 +++ b/ScriptMessage/Services/MicrosoftGraph.ps1 @@ -1074,7 +1074,9 @@ function Send-ScriptMessage_MicrosoftGraph } # Announce this service to the module. Keep this at the bottom of the file, after the functions it names. +# 'MailType' is absent from SupportedSetting on purpose: nothing here acts on it. Register-ScriptMessageService -Name 'MicrosoftGraph' ` + -SupportedSetting 'ChatType', 'IncludeBCCInGroupChat' ` -ConnectFunction 'Connect-ScriptMessage_MicrosoftGraph' ` -DisconnectFunction 'Disconnect-ScriptMessage_MicrosoftGraph' ` -SendFunction 'Send-ScriptMessage_MicrosoftGraph' ` diff --git a/Tests/ServiceDispatch.Tests.ps1 b/Tests/ServiceDispatch.Tests.ps1 index b45cda5..53ddf29 100644 --- a/Tests/ServiceDispatch.Tests.ps1 +++ b/Tests/ServiceDispatch.Tests.ps1 @@ -31,11 +31,69 @@ Describe 'The service table' { It 'refuses to register a service that is not a MessagingService member' { InModuleScope ScriptMessage { - { Register-ScriptMessageService -Name 'Carrier Pigeon' ` + { Register-ScriptMessageService -Name 'Carrier Pigeon' -SupportedSetting @() ` -ConnectFunction 'c' -DisconnectFunction 'd' -SendFunction 's' -GetContextFunction 'g' } | Should -Throw '*not a member of the MessagingService enum*' } } + + It 'records which generic settings Microsoft Graph honors, and which it does not' { + InModuleScope ScriptMessage { + $Supported = (Get-ScriptMessageServiceRegistration -Service 'MicrosoftGraph').SupportedSetting + + $Supported | Should -Contain 'ChatType' + $Supported | Should -Contain 'IncludeBCCInGroupChat' + $Supported | Should -Not -Contain 'MailType' + } + } +} + +Describe 'A generic setting the service does not support' { + BeforeAll { + # Pester can only mock a command that exists, so the stand-in has to come before the mock. + InModuleScope ScriptMessage { + function script:Send-MgUserMail { param($UserId, $BodyParameter, [switch]$PassThru) throw 'Send-MgUserMail stand-in called without a mock.' } + } + + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + ChatType = 'Group' + IncludeBCCInGroupChat = $false + MgPermissionType = 'Application' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + Mock -ModuleName ScriptMessage Connect-ScriptMessage { } + Mock -ModuleName ScriptMessage Send-MgUserMail { $true } + + $MailArguments = @{ + Service = 'MicrosoftGraph' + Type = 'Mail' + From = 'sender@example.org' + To = 'recipient@example.org' + Subject = 'Test subject' + Body = 'Test body' + } + } + + It 'reports -MailType as unsupported and still sends' { + $Warnings = @() + $null = Send-ScriptMessage @MailArguments -MailType Group -WarningVariable Warnings -WarningAction SilentlyContinue + + ($Warnings | Out-String) | Should -BeLike "*does not support the 'MailType' parameter*" + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly + } + + It 'says nothing about a setting the service does support' { + $Warnings = @() + $null = Send-ScriptMessage @MailArguments -ChatType Group -WarningVariable Warnings -WarningAction SilentlyContinue + + ($Warnings | Out-String) | Should -Not -BeLike '*does not support*' + } } Describe 'The public cmdlets dispatch through the service table' { From 6c6455ba3c0dcbdc969eb7d57d078e8697c15c22 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:30:19 -0500 Subject: [PATCH 09/10] Implement MailType and read boolean settings as opt-in flags - -MailType and the configuration file's MailType setting were accepted and ignored. OneOnOne now sends each unique To, CC, and BCC address its own email with only that address in To; Group, the default, sends one email. Send-ScriptMessage resolves it like ChatType: the parameter beats the configuration file, a missing or blank setting means Group, and an unrecognized value throws before connecting. It is resolved only for a send that includes Mail, so it cannot stop a Chat send. - The Graph service sends one request per recipient, deduplicating addresses without regard to case. A failure for one recipient goes into the result's Error property and the rest are still sent; -ErrorAction Stop makes the catch fire whether or not Send-MgUserMail's error is terminating (Research_Notes/Microsoft-Graph-Behavior.md section 3). The result's MailType property now holds the mail type used. Microsoft Graph declares MailType in -SupportedSetting, and ServiceDispatch.Tests.ps1 narrows the registration to keep exercising the unsupported-setting warning. - Remove Get-ScriptMessageBooleanSetting. The 1.1.0 bug was two reads that turn a quoted "false" into true: a [bool] cast for IncludeBCCInGroupChat and a truthiness test for MgDisconnectWhenDone. Every boolean setting is now an opt-in flag read as -eq $true with the setting on the left, the form the MgDelegatedPermission_* settings already used, so any value other than true means off, the safe state for all four. The rule is in AGENTS.md and the measurements in Research_Notes/PowerShell-Language-Behavior.md section 10. - Add Tests/FileEncoding.Tests.ps1, which enforces the encoding rules in AGENTS.md (no BOM except a manifest that needs one, LF only, ASCII-only PowerShell files) over tracked and untracked, unignored files, and replace the manual check snippet in AGENTS.md with a pointer to it. - Add a Testing section to AGENTS.md: running Invoke-Tests.ps1, how the Pester files avoid a tenant, and that the scratch scripts in Tests/ send real messages. - RELEASING.md: develop allows squash merges for feature branches, but main must be merged back into develop with a merge commit so develop keeps the tagged release commit. - Changelog: add -MailType under Added, drop the entry for the -MailType warning (which never shipped) and the help-text entry, drop the specific quoted-boolean entries, and add a general Minor entry about validation. --- AGENTS.md | 47 ++-- CHANGELOG.md | 11 +- CONTRIBUTING.md | 3 +- RELEASING.md | 18 +- Research_Notes/Microsoft-Graph-Behavior.md | 11 + .../PowerShell-Language-Behavior.md | 37 ++- .../Get-ScriptMessageBooleanSetting.ps1 | 33 --- ScriptMessage/Public/Send-ScriptMessage.ps1 | 43 ++- ScriptMessage/Services/MicrosoftGraph.ps1 | 86 +++++- Tests/FileEncoding.Tests.ps1 | 88 +++++++ Tests/Send-ScriptMessage.Tests.ps1 | 247 ++++++++++++++++-- Tests/ServiceDispatch.Tests.ps1 | 22 +- 12 files changed, 536 insertions(+), 110 deletions(-) delete mode 100644 ScriptMessage/Private/Get-ScriptMessageBooleanSetting.ps1 create mode 100644 Tests/FileEncoding.Tests.ps1 diff --git a/AGENTS.md b/AGENTS.md index 1ca8ec5..75927d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -181,28 +181,15 @@ without going to look. it and corrupts it. Git will not show you a line-ending mistake: it normalizes before diffing, so a wrong file produces no diff -and leaves `git status` clean. Use this instead, which wants `w/lf` on every row: +and leaves `git status` clean. **`Tests/FileEncoding.Tests.ps1` enforces all of these rules** as part of the +test suite: no byte order mark (a module manifest that holds a non-ASCII character is the one exception), LF line +endings, and only ASCII in PowerShell files. It checks untracked files that are not ignored as well as tracked +ones, so it catches a new file before it is committed, and it skips any file containing a NUL byte. A failure +names each offending file. -```powershell -git ls-files --eol -- '*.ps1' '*.psm1' '*.psd1' '*.md' '*.json' '*.yml' -``` - -Nothing enforces the BOM and ASCII rules automatically yet, so check them before committing. Run this from the -repository root; it prints nothing when every tracked file passes. A module manifest saved with a BOM on purpose -is reported, and is the one expected exception: - -```powershell -git ls-files | ForEach-Object { - $Path = Join-Path $PWD.ProviderPath $_ # .NET does not follow PowerShell's current location - $Bytes = [System.IO.File]::ReadAllBytes($Path) - if ($Bytes.Length -ge 3 -and $Bytes[0] -eq 0xEF -and $Bytes[1] -eq 0xBB -and $Bytes[2] -eq 0xBF) { "BOM: $_" } - if (($_ -match '\.(ps1|psm1|psd1|ps1xml)$') -and ([System.IO.File]::ReadAllText($Path) -match '[^\x00-\x7F]')) { "Non-ASCII: $_" } -} -``` - -New files can pick up CRLF on Windows (VS Code's default line ending follows the operating system), so check -a file you created with the `git ls-files --eol` command above once it is tracked. Never change the line -endings of a file you are not otherwise editing. To fix one that is wrong: +New files can pick up CRLF on Windows (VS Code's default line ending follows the operating system), so run the +tests after creating a file, not only after changing code. Never change the line endings of a file you are not +otherwise editing. To fix one that is wrong: ```powershell $Path = (Resolve-Path -LiteralPath '').ProviderPath # .NET needs a full path @@ -218,6 +205,24 @@ $Text = [System.IO.File]::ReadAllText($Path) `AliasesToExport`. - **Comments describe the code as it is now**, never what it used to do or what a fix changed. Change history belongs in the commit message and the changelog. +- **A boolean configuration setting is an opt-in flag: on only when it equals `$true`.** Read it as + `$ServiceConfig. -eq $true`, with the setting on the left, so missing, `false`, quoted text, and typos + all mean off. Never read one with a `[bool]` cast, a truthiness test, or `$true -eq`: each of those reads the + text `"false"` as true. Name a new boolean setting so that `true` opts in and off is the safe state. + +## Testing + +- **Run `.\Tests\Invoke-Tests.ps1` from the repository root before proposing a commit.** It runs every + `Tests/*.Tests.ps1` file under both Windows PowerShell 5.1 and PowerShell 7, each in a fresh process, and the + CI workflow runs the same script. `-Edition Core` runs PowerShell 7 alone for a quicker check; finish with both, + because the editions behave differently in places. +- **The Pester files never connect to a tenant** and need no Microsoft Graph modules. Mock the service functions + such as `Send-ScriptMessage_MicrosoftGraph`, or the Microsoft Graph cmdlets they call. Pester can only mock a + command that exists, so a test that mocks a Graph cmdlet first defines a stand-in for it inside the module that + throws if reached, as the existing files do. +- **The other scripts in `Tests/` are not tests.** `ScriptMessage_Testing.ps1` and + `ScriptMessage_Testing (Installed Module).ps1` are scratch scripts that load the real configuration from + `Tests/Config/`, so anything run from them sends real messages; the safety rules above apply. ## Research notes diff --git a/CHANGELOG.md b/CHANGELOG.md index acce9a1..bee4a67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,17 +2,20 @@ ## [Unreleased](https://github.com/Sekers/ScriptMessage/compare/1.1.1...develop) +### Added + +- New Parameter: `Send-ScriptMessage -MailType` > Chooses how an email with more than one recipient is sent. It overrides the configuration file's `MailType` setting, and `Group` is used when neither one sets it. + - `Group` sends one email to all of the To, CC, and BCC recipients. + - `OneOnOne` sends each To, CC, and BCC recipient a separate email with only that recipient in To, so no recipient can see who else received it. A recipient listed more than once gets one email. + ### Fixed -- `Send-ScriptMessage` now requires the `IncludeBCCInGroupChat` setting to be an unquoted `true` or `false`, not a quoted string. Quotes make the value text, and any non-empty text was read as `true`. So a configuration file holding `"IncludeBCCInGroupChat": "false"` added BCC recipients to a group chat and made their addresses visible to everyone in it, and passing the string `'false'` to `-IncludeBCCInGroupChat` did the same. Both now stop with an error naming the setting or the parameter; remove the quotes in the configuration file, and pass `$false` to the parameter. -- `Send-ScriptMessage` now requires the `MgDisconnectWhenDone` setting to be an unquoted `true` or `false`, not a quoted string. Quotes make the value text, which was read as `true`, so a configuration file holding `"MgDisconnectWhenDone": "false"` disconnected from Microsoft Graph after sending, the opposite of what the setting asks for. It now stops with an error naming the setting; remove the quotes. - `Send-ScriptMessage` now honors an explicit `-ChatType OneOnOne`. The configuration file's `ChatType` setting was used in its place, so a call asking for one-on-one chats sent a single group chat when that setting was `Group`. An explicit `-ChatType Group` was unaffected. - `Send-ScriptMessage` now honors an explicit `-IncludeBCCInGroupChat $false`. The configuration file's `IncludeBCCInGroupChat` setting was used in its place, so a call asking to keep BCC recipients out of a group chat added them anyway when that setting was `true`, making those addresses visible to everyone in the chat. An explicit `-IncludeBCCInGroupChat $true` was unaffected. - `Send-ScriptMessage` now sends mail without a `ChatType` setting in the configuration file. A Mail-only send failed without it, reporting "Cannot convert null to type ChatType", even though `ChatType` only affects chat; a configuration file listing only `Mail` in `AllowableMessageTypes`, or one written before the setting was added in version 1.0.8, was affected. A `Chat` send with no chat type, because the setting is missing or blank and `-ChatType` was not passed, now reports that in the returned result's `Error` property and names both places it can be set. - `Get-ScriptMessageContext -ReturnCachedContext` no longer reports a connection that `Disconnect-ScriptMessage` has already ended. Disconnecting left the cached context in place, so a cached read kept returning the account and scopes of the finished session, and only a read without `-ReturnCachedContext` showed the truth. Disconnecting now clears that service's cached context, so the next cached read asks the service again. -- `Send-ScriptMessage` now reports that `-MailType` was ignored instead of accepting it in silence. No messaging service acts on that parameter, so a call passing it behaved as though it had not, and the returned result's `MailType` property is empty for the same reason. The value is still ignored, but the call now warns which service does not support it and carries on sending. +- Minor: Improved how `Send-ScriptMessage` validates configuration file settings, so a value in a form a setting does not support is no longer misread. - Minor: The help for `Get-ScriptMessageConfig` and `Set-ScriptMessageConfigFilePath` now describes the `-Path` parameter, which it left blank. -- Minor: `Get-Help Send-ScriptMessage -Parameter MailType` now describes the parameter. It returned an internal note reading "TODO: Implement MailType (similar to ChatType so we can do 1:1 emailing)". - Minor: The fourth `Send-ScriptMessage` help example, which sends attachments held in variables, now runs as written. It failed with "Missing closing ')' in subexpression" when copied, and it ended without calling `Send-ScriptMessage`. --- diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7af6c61..fdcc96f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -30,7 +30,8 @@ Thank you for helping improve ScriptMessage. This guide covers the everyday work your own test tenant and describe what you tested in the pull request. - **Text files are UTF-8 with LF line endings and no byte order mark, and PowerShell files contain only ASCII.** Windows PowerShell 5.1 misreads non-ASCII characters in PowerShell files saved without a byte order mark. - `.gitattributes` handles line endings, and [AGENTS.md](./AGENTS.md) explains the rules and how to check them. + `.gitattributes` handles line endings, the test suite checks all three rules, and [AGENTS.md](./AGENTS.md) + explains them. - **Rebase only your own branch, and only before it merges.** Never force-push `develop` or `main`. AI assistants working in this repository follow [AGENTS.md](./AGENTS.md), which also holds the detailed rules diff --git a/RELEASING.md b/RELEASING.md index 67c785d..0f56b27 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -95,7 +95,12 @@ Rules for the shared branches: exist on `develop`, so every later release pull request would conflict with it. - **Never rebase or force-push `main` or `develop`.** Release tags cannot move, and rewriting shared history breaks every contributor's clone. Rebase only your own feature branch, and only before it merges. -- **After every release or hotfix, merge `main` back into `develop`**, so `develop` contains the release commit. +- **After every release or hotfix, merge `main` back into `develop` with a merge commit, never a squash or a + rebase**, so `develop` contains the tagged release commit. A squash or a rebase copies `main`'s changes into + new commits instead, so `develop` never contains the tagged commit, and a later release pull request can + conflict with a hotfix that `develop` already holds under a different commit. `develop` also allows squash + merges, for consolidating a feature branch, and GitHub's merge button offers the method used last, so check it + before merging `main` back. - **Stable release tags go only on `main`.** Prerelease tags may go on `develop`. ## Changelog @@ -139,7 +144,7 @@ section. 3. Builds the package once and publishes it to the PowerShell Gallery, in the `psgallery` environment. 4. Creates the GitHub release from the version's changelog section, attaches the same package, and publishes the release. -6. **Merge `main` back into `develop`.** +6. **Merge `main` back into `develop` with a merge commit**, not a squash ([Branches](#branches) explains why). The GitHub release is created last so that a failed check or publish never leaves behind a public release, which immutable releases would lock, for a version that did not ship. @@ -158,8 +163,8 @@ immutable releases would lock, for a version that did not ship. 2. Fix the problem, add tests, bump the patch version, and add a dated changelog section for the hotfix. 3. Open a pull request into `main`, merge it with a merge commit, and tag and push as in [Releasing a version](#releasing-a-version), steps 4 and 5. -4. Merge `main` into `develop`. In `CHANGELOG.md`, keep the `[Unreleased]` section from `develop` at the top, - with the hotfix's section below it. +4. Merge `main` into `develop` with a merge commit. In `CHANGELOG.md`, keep the `[Unreleased]` section from + `develop` at the top, with the hotfix's section below it. ## Prereleases @@ -174,7 +179,8 @@ A repository admin configures these once in the GitHub repository settings. They enforceable: - **Branch rulesets for `main` and `develop`:** require a pull request, require the `Workflow lint` and `Pester` - checks to pass, and block force pushes and deletion. For `main`, allow only merge commits. + checks to pass, and block force pushes and deletion. For `main`, allow only merge commits. For `develop`, also + allow squash merges, for consolidating a feature branch. - **A tag ruleset for release tags:** target tags that look like versions, and block updates and deletion, with bypass only for admins. - **Immutable releases:** enabled, so a published release's tag and assets cannot change. @@ -194,6 +200,6 @@ enforceable: - [ ] The tag is annotated, has no `v` prefix, and is on the merge commit. - [ ] Every job in the Release workflow passed. - [ ] The version appears on the PowerShell Gallery, and the GitHub release has the right notes and package. -- [ ] `main` was merged back into `develop`. +- [ ] `main` was merged back into `develop` with a merge commit. - [ ] Anything that needs a live Microsoft Graph test was tested separately in a test tenant. The automated tests never connect to a tenant. diff --git a/Research_Notes/Microsoft-Graph-Behavior.md b/Research_Notes/Microsoft-Graph-Behavior.md index 7d87b49..f8d345b 100644 --- a/Research_Notes/Microsoft-Graph-Behavior.md +++ b/Research_Notes/Microsoft-Graph-Behavior.md @@ -89,6 +89,17 @@ the module has no way to report a later delivery failure. **Unverified:** that `Send-MgUserMail -PassThru` returns only `$true`. A comment in the module says so; it was not checked. +**From source:** with `MailType` `Group`, `Send-ScriptMessage_MicrosoftGraph` calls `Send-MgUserMail` once, with +no `-ErrorAction`, inside a `try` whose `catch` puts the error in the result's `Error` property. With `OneOnOne` +it calls `Send-MgUserMail` once per unique recipient with `-ErrorAction Stop`, so each failure reaches that +recipient's own `catch` and the remaining recipients are still sent. + +**Unverified:** whether a failed `Send-MgUserMail` raises a terminating error. The `Group` send depends on it: if +the error is non-terminating, it goes to the caller's error stream instead of `Error`, and `Status` is empty. + +**Unverified:** how Exchange Online throttles many `sendMail` requests from one mailbox in quick succession, +which a `OneOnOne` send to a long recipient list makes. The sendMail page quoted above gives no figures. + ## 4. From documentation: creating one-on-one and group chats The Create chat page: "Only one one-on-one chat can exist between two members. If a one-on-one chat already diff --git a/Research_Notes/PowerShell-Language-Behavior.md b/Research_Notes/PowerShell-Language-Behavior.md index b7343f5..92891b7 100644 --- a/Research_Notes/PowerShell-Language-Behavior.md +++ b/Research_Notes/PowerShell-Language-Behavior.md @@ -282,17 +282,33 @@ an empty string. `Send-ScriptMessage_MicrosoftGraph` then reports a Chat message `IncludeBCCInGroupChat` needs no such guard: it is resolved through a `[bool]`-constrained variable inside a function, so a configuration file with no `IncludeBCCInGroupChat` setting yields `False`. -The parameter binder is stricter than a cast or a validation set, and that difference is load-bearing. A -`[bool]` or `[Nullable[bool]]` parameter refuses a string, while `[Object]` with `[ValidateSet($null, $true, -$false)]` accepts `'false'` and leaves it a string for a later `[bool]` cast to turn into `True`. A -configuration file value never passes through a binder at all, so it needs its own check. +The parameter binder is stricter than a cast or a validation set. A `[bool]` or `[Nullable[bool]]` parameter +refuses a string, while `[Object]` with `[ValidateSet($null, $true, $false)]` accepts `'false'` and leaves it a +string for a later `[bool]` cast to turn into `True`. A configuration file value never passes through a binder +at all. + +Comparing with `-eq` depends on which side the value is on. Measured on **2026-09-21**, same editions, both +agreeing: + +| Value | `$value -eq $true` | `$true -eq $value` | +| --- | --- | --- | +| `'true'` | `True` | `True` | +| `'false'` | `False` | `True` | +| `'yes'` | `False` | `True` | +| `$null` | `False` | `False` | +| `$false` / `$true` | `False` / `True` | `False` / `True` | + +With the string on the left, `-eq` converts `$true` to the text `True` and compares without regard to case, so +only text reading "true" matches. With `$true` on the left, it converts the string to `[bool]`, which makes every +non-empty string `True`, exactly as a cast does. **From source:** `Send-ScriptMessage` declares `-IncludeBCCInGroupChat` as `[Nullable[bool]]`, which keeps the -three states it needs (unset, true, false) and makes the binder reject a string. A configuration file value -reaches no binder, so `Get-ScriptMessageBooleanSetting` rejects any value that is `-isnot [bool]`. Each layer -calls it for the settings it owns and before anything is sent: `Send-ScriptMessage` for the generic -`IncludeBCCInGroupChat`, and `Send-ScriptMessage_MicrosoftGraph` for its own `MgDisconnectWhenDone`, ahead of -that function's message type loop. Unquoted JSON `true` and `false` arrive as real booleans and are unaffected. +three states it needs (unset, true, false) and makes the binder reject a string. Every boolean configuration +setting is read as an opt-in flag, ` -eq $true` with the setting on the left: `IncludeBCCInGroupChat` in +`Send-ScriptMessage`, `MgDisconnectWhenDone` in `Send-ScriptMessage_MicrosoftGraph`, and +`MgDelegatedPermission_RequestChatReadPermission` and `MgDelegatedPermission_RequestFilesReadWritePermission` in +`Connect-ScriptMessage_MicrosoftGraph`. A missing setting, `false`, quoted text, and a typo all read as off, which +is the safe state of all four. Unquoted JSON `true` and `false` arrive as real booleans. ### What this does not establish @@ -302,3 +318,6 @@ that function's message type loop. Unquoted JSON `true` and `false` arrive as re - `[Nullable[bool]]` and `[CT?]` parameters were not tested here; section 1 covers an unassigned `[Nullable[bool]]` variable only. - Only `$null` was tested as the rejected value. Empty strings and other unconvertible values were not. +- Numbers were measured only with the value on the left, the same day: `1 -eq $true` is `True` and + `0 -eq $true` is `False`. A JSON array or object in a boolean setting was not tested; an array on the left of + `-eq` filters rather than compares. diff --git a/ScriptMessage/Private/Get-ScriptMessageBooleanSetting.ps1 b/ScriptMessage/Private/Get-ScriptMessageBooleanSetting.ps1 deleted file mode 100644 index d0db95b..0000000 --- a/ScriptMessage/Private/Get-ScriptMessageBooleanSetting.ps1 +++ /dev/null @@ -1,33 +0,0 @@ -function Get-ScriptMessageBooleanSetting -{ - [CmdletBinding()] - param( - [Parameter( - Mandatory = $true, - ValueFromPipeline = $true, - ValueFromPipelineByPropertyName = $true)] - [AllowNull()] - [Object]$Value, - - [Parameter( - Mandatory = $true, - ValueFromPipelineByPropertyName = $true)] - [string]$Name - ) - - # An unset setting stays unset. The caller decides what that means for its own setting. - if ($null -eq $Value) - { - return $null - } - - # A JSON boolean arrives as a [bool]. A quoted "true" or "false" arrives as a string, and casting a string - # to [bool] makes every non-empty one $true, so a quoted "false" would mean its opposite. Reject anything - # that is not already a boolean rather than guessing at it. - if ($Value -isnot [bool]) - { - throw "The `'$Name`' setting in the ScriptMessage configuration file must be a boolean written as true or false without quotes. Found the $($Value.GetType().Name) value `'$Value`'." - } - - return $Value -} diff --git a/ScriptMessage/Public/Send-ScriptMessage.ps1 b/ScriptMessage/Public/Send-ScriptMessage.ps1 index 5794b78..fa28a77 100644 --- a/ScriptMessage/Public/Send-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Send-ScriptMessage.ps1 @@ -51,7 +51,9 @@ function Send-ScriptMessage If not specified, defaults to the address inside of the 'From' parameter. .PARAMETER MailType Override the default 'MailType' specified in the configuration file for the messaging service being used. Options are 'OneOnOne' or 'Group'. - No messaging service supports this parameter yet, so a value given here is reported as unsupported and ignored. + 'Group' sends one message to all of the To, CC, and BCC recipients. 'OneOnOne' sends each recipient a separate message with only that recipient in To, so no recipient can see who else received it. A recipient listed more than once, even across To, CC, and BCC, receives one message. + Each message counts separately against the messaging service's sending limits. + If neither this parameter nor the configuration file sets a mail type, 'Group' is used. .PARAMETER ChatType Override the default 'ChatType' specified in the configuration file for the messaging service being used. Options are 'OneOnOne' or 'Group'. .PARAMETER IncludeBCCInGroupChat @@ -230,7 +232,6 @@ function Send-ScriptMessage ValueFromPipelineByPropertyName = $true)] [string]$SenderId, - # TODO: Implement MailType (similar to ChatType so we can do 1:1 emailing) [Parameter( Mandatory = $false, ValueFromPipeline = $true, @@ -340,15 +341,42 @@ function Send-ScriptMessage } # 'IncludeBCCInGroupChat' accepts $null to mean "use the configured value", which is also what an - # unsupplied [Nullable[bool]] holds. + # unsupplied [Nullable[bool]] holds. The configured value is an opt-in flag, on only when it equals + # $true, so anything else, including quoted text, leaves BCC recipients out. Keep the setting on the + # left: a [bool] cast, or $true on the left, reads the text "false" as $true. if ($null -ne $IncludeBCCInGroupChat) { [bool]$ResolvedIncludeBCCInGroupChat = $IncludeBCCInGroupChat } else { - [bool]$ResolvedIncludeBCCInGroupChat = Get-ScriptMessageBooleanSetting -Name 'IncludeBCCInGroupChat' ` - -Value $ConnectionParameters.ServiceConfig.IncludeBCCInGroupChat + [bool]$ResolvedIncludeBCCInGroupChat = ($ConnectionParameters.ServiceConfig.IncludeBCCInGroupChat -eq $true) + } + + # 'MailType' is resolved only for a send that includes Mail, so a configuration value that only Mail would + # use cannot stop a Chat send. 'Group' applies when neither the parameter nor the configuration file sets + # one, including the empty string the configuration template uses for a setting it leaves unset, so a + # configuration file without the setting sends one message to every recipient. + $ResolvedMailType = $null + if ($serviceTypeObj.Type -contains [MessageType]::Mail) + { + $ConfiguredMailType = $ConnectionParameters.ServiceConfig.MailType + if ($PSBoundParameters.ContainsKey('MailType')) + { + $ResolvedMailType = $MailType + } + elseif ([string]::IsNullOrWhiteSpace($ConfiguredMailType)) + { + $ResolvedMailType = [MailType]::Group + } + elseif ($ConfiguredMailType -in [Enum]::GetNames([MailType])) + { + $ResolvedMailType = [MailType]$ConfiguredMailType + } + else + { + throw "The `'MailType`' setting in the ScriptMessage configuration file must be one of: $([Enum]::GetNames([MailType]) -join ', '). Found `'$ConfiguredMailType`'." + } } # Connect to the messaging service, if necessary (e.g., API service). @@ -378,6 +406,11 @@ function Send-ScriptMessage $SendMessageParameters['ChatType'] = $ResolvedChatType } + if ($null -ne $ResolvedMailType) + { + $SendMessageParameters['MailType'] = $ResolvedMailType + } + & $Registration.SendFunction @SendMessageParameters } } \ No newline at end of file diff --git a/ScriptMessage/Services/MicrosoftGraph.ps1 b/ScriptMessage/Services/MicrosoftGraph.ps1 index 9bcff88..18324f5 100644 --- a/ScriptMessage/Services/MicrosoftGraph.ps1 +++ b/ScriptMessage/Services/MicrosoftGraph.ps1 @@ -502,6 +502,12 @@ function Send-ScriptMessage_MicrosoftGraph ValueFromPipelineByPropertyName = $true)] [string]$SenderId, + [Parameter( + Mandatory = $false, + ValueFromPipeline = $true, + ValueFromPipelineByPropertyName = $true)] + [MailType]$MailType = 'Group', + [Parameter( Mandatory = $false, ValueFromPipeline = $true, @@ -530,11 +536,6 @@ function Send-ScriptMessage_MicrosoftGraph $ServiceConfig = Get-ScriptMessageConfig -Service $ServiceId } - # This service's own boolean settings, checked before anything is sent so a value the module cannot read - # stops the send instead of surfacing after the message has already gone out. - $MgDisconnectWhenDone = Get-ScriptMessageBooleanSetting -Name 'MgDisconnectWhenDone' ` - -Value $ServiceConfig.MgDisconnectWhenDone - # Send the message on each supported service specified. foreach ($typeItem in $Type) { @@ -597,9 +598,70 @@ function Send-ScriptMessage_MicrosoftGraph { $SenderId = $Message.From.emailAddress.Address # Note: This is correct as 'xxxx.Address' (not 'AddressObj'). It is converted Microsoft's to IMicrosoftGraphRecipient. } - + # Send Email. - $SendEmailMessageResult = Send-MgUserMail -UserId $SenderId -BodyParameter $EmailParams -PassThru + switch ($MailType) + { + Group + { + $SendEmailMessageResult = Send-MgUserMail -UserId $SenderId -BodyParameter $EmailParams -PassThru + } + OneOnOne + { + # Each recipient gets a message with only them in To, so nobody sees who else was sent + # one. An address listed more than once, even across To, CC, and BCC, gets one message; + # hashtable keys compare without case, as email addresses do. + $IndividualRecipients = [System.Collections.Generic.List[Object]]::new() + $SeenAddresses = @{} + foreach ($recipient in @($Message.To) + @($Message.CC) + @($Message.BCC)) + { + $RecipientAddress = $recipient.EmailAddress.Address + if ([string]::IsNullOrWhiteSpace($RecipientAddress) -or $SeenAddresses.ContainsKey($RecipientAddress)) + { + continue + } + $SeenAddresses[$RecipientAddress] = $true + $IndividualRecipients.Add($recipient) + } + + # One recipient's failure is reported and the rest are still sent, so Status is $true only + # when every message went out. -ErrorAction Stop makes a failure reach the catch whether or + # not the cmdlet reports it as a terminating error. + $SentCount = 0 + foreach ($recipient in $IndividualRecipients) + { + # A new request for each recipient rather than one edited in place, so no two + # requests share an object. + $IndividualMessage = [ordered]@{} + foreach ($field in $EmailParams.Message.Keys) + { + $IndividualMessage[$field] = $EmailParams.Message[$field] + } + $IndividualMessage['ToRecipients'] = @($recipient) + $IndividualMessage['CcRecipients'] = $null + $IndividualMessage['BccRecipients'] = $null + $IndividualEmailParams = [ordered]@{ + SaveToSentItems = $EmailParams.SaveToSentItems + Message = $IndividualMessage + } + + try + { + $null = Send-MgUserMail -UserId $SenderId -BodyParameter $IndividualEmailParams -PassThru -ErrorAction Stop + $SentCount++ + } + catch + { + $MgErrorMessages += "Mail not sent to `'$($recipient.EmailAddress.Address)`': $_" + } + } + + if ($SentCount -eq $IndividualRecipients.Count) + { + $SendEmailMessageResult = $true + } + } + } } } catch @@ -684,7 +746,7 @@ function Send-ScriptMessage_MicrosoftGraph $SendScriptMessageResult = [PSCustomObject]@{ MessageService = $ServiceId MessageType = $typeItem - MailType = $MailType # TODO: MAILTYPE + MailType = $MailType Status = $SendEmailMessageResult # The SDK only returns $true and nothing else (and only that because of the 'PassThru') Error = $SendScriptMessageResult_Error SentFrom = $SendScriptMessageResult_SentFrom @@ -1066,17 +1128,17 @@ function Send-ScriptMessage_MicrosoftGraph } } - # Disconnect from the Microsoft Graph API, if enabled in the configuration file. - if ($MgDisconnectWhenDone) + # Disconnect from the Microsoft Graph API, if enabled in the configuration file. The setting is an opt-in flag, + # on only when it equals $true; keep it on the left, because a truthiness test reads the text "false" as $true. + if ($ServiceConfig.MgDisconnectWhenDone -eq $true) { $null = Disconnect-MgGraph -ErrorAction SilentlyContinue } } # Announce this service to the module. Keep this at the bottom of the file, after the functions it names. -# 'MailType' is absent from SupportedSetting on purpose: nothing here acts on it. Register-ScriptMessageService -Name 'MicrosoftGraph' ` - -SupportedSetting 'ChatType', 'IncludeBCCInGroupChat' ` + -SupportedSetting 'MailType', 'ChatType', 'IncludeBCCInGroupChat' ` -ConnectFunction 'Connect-ScriptMessage_MicrosoftGraph' ` -DisconnectFunction 'Disconnect-ScriptMessage_MicrosoftGraph' ` -SendFunction 'Send-ScriptMessage_MicrosoftGraph' ` diff --git a/Tests/FileEncoding.Tests.ps1 b/Tests/FileEncoding.Tests.ps1 new file mode 100644 index 0000000..590e724 --- /dev/null +++ b/Tests/FileEncoding.Tests.ps1 @@ -0,0 +1,88 @@ +# Pester 5 or later. Run .\Tests\Invoke-Tests.ps1 to test under both PowerShell editions, or +# Invoke-Pester -Path .\Tests for the current one. Nothing here imports the module or connects to a tenant: it +# reads the repository's files from disk and checks them against the file encoding rules in AGENTS.md. +# +# Git will not report these problems itself. It normalizes line endings before diffing, so a file whose endings +# are wrong produces no diff, and it says nothing about a byte order mark or a non-ASCII character. + +BeforeAll { + $RepoRoot = (Resolve-Path -LiteralPath (Join-Path $PSScriptRoot '..')).ProviderPath + + # Tracked files plus untracked files that are not ignored, so a new file is checked before it is committed + # while .gitignore (which keeps real configuration files out) is still honored. -z stops Git quoting paths. + $Listed = @((& git -C $RepoRoot ls-files -z --cached --others --exclude-standard) -join '' -split "`0" | + Where-Object { $_ } | Sort-Object -Unique) + if (($LASTEXITCODE -ne 0) -or ($Listed.Count -eq 0)) + { + throw "git ls-files listed nothing under '$RepoRoot' (exit code $LASTEXITCODE). These tests need Git and a clone of the repository." + } + + # ISO-8859-1 maps each byte to the character with the same number, so a regex over the result sees the raw + # bytes, and a byte above 0x7F is a non-ASCII character in any encoding this repository could hold. + $Latin1 = [System.Text.Encoding]::GetEncoding(28591) + + $Checked = [System.Collections.Generic.List[string]]::new() + $WithBom = [System.Collections.Generic.List[string]]::new() + $WithCR = [System.Collections.Generic.List[string]]::new() + $NonAsciiPowerShell = [System.Collections.Generic.List[string]]::new() + + foreach ($Relative in $Listed) + { + $Path = Join-Path $RepoRoot $Relative + if (-not (Test-Path -LiteralPath $Path -PathType Leaf)) + { + continue # Tracked, but deleted in the working tree. + } + + $Bytes = [System.IO.File]::ReadAllBytes($Path) + + # A NUL byte is Git's own test for a binary file. Git applies no line-ending rule to one, and none of + # these rules apply either. + if ([Array]::IndexOf($Bytes, [byte]0) -ge 0) + { + continue + } + $Checked.Add($Relative) + + $HasBom = ($Bytes.Length -ge 3) -and ($Bytes[0] -eq 0xEF) -and ($Bytes[1] -eq 0xBB) -and ($Bytes[2] -eq 0xBF) + $ContentStart = if ($HasBom) { 3 } else { 0 } + $HasNonAscii = $Latin1.GetString($Bytes, $ContentStart, $Bytes.Length - $ContentStart) -match '[^\x00-\x7F]' + + # The one exception to both rules: a module manifest cannot write a non-ASCII character as an escape, so + # one that needs such a character is saved with a BOM, which both editions read correctly. + $IsManifestNeedingBom = ($Relative -like '*.psd1') -and $HasBom -and $HasNonAscii + + if ($HasBom -and -not $IsManifestNeedingBom) + { + $WithBom.Add($Relative) + } + if ([Array]::IndexOf($Bytes, [byte]13) -ge 0) + { + $WithCR.Add($Relative) + } + if (($Relative -match '\.(ps1|psm1|psd1|ps1xml)$') -and $HasNonAscii -and -not $IsManifestNeedingBom) + { + $NonAsciiPowerShell.Add($Relative) + } + } +} + +Describe 'Repository file encoding' { + # A check that silently reads nothing would pass every test below. + It 'reads the repository''s text files' { + $Checked | Should -Contain 'ScriptMessage/ScriptMessage.psm1' + $Checked | Should -Contain 'CHANGELOG.md' + } + + It 'no text file starts with a byte order mark' { + $WithBom | Should -BeNullOrEmpty -Because 'text files are UTF-8 without a BOM, except a module manifest that holds a non-ASCII character' + } + + It 'every text file uses LF line endings' { + $WithCR | Should -BeNullOrEmpty -Because '.gitattributes pins every text file to eol=lf; AGENTS.md shows how to convert one' + } + + It 'PowerShell files contain only ASCII' { + $NonAsciiPowerShell | Should -BeNullOrEmpty -Because 'Windows PowerShell 5.1 misreads a non-ASCII character in a file without a BOM; write it as an escape such as [char]0x00E9' + } +} diff --git a/Tests/Send-ScriptMessage.Tests.ps1 b/Tests/Send-ScriptMessage.Tests.ps1 index 071a79d..bc94774 100644 --- a/Tests/Send-ScriptMessage.Tests.ps1 +++ b/Tests/Send-ScriptMessage.Tests.ps1 @@ -302,6 +302,219 @@ Describe 'Send-ScriptMessage mail attachment' { } } +Describe 'Send-ScriptMessage mail type' { + BeforeAll { + Mock -ModuleName ScriptMessage Connect-ScriptMessage { } + Mock -ModuleName ScriptMessage Send-MgUserMail { $true } + + $MailArguments = @{ + Service = 'MicrosoftGraph' + Type = 'Mail' + From = 'sender@example.org' + To = @('a@example.org', 'b@example.org') + CC = 'c@example.org' + BCC = 'd@example.org' + Subject = 'Test subject' + Body = 'Test body' + } + } + + Context 'A configuration that sets MailType to Group' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + MailType = 'Group' + MgPermissionType = 'Application' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'sends one message to every recipient' { + $Result = Send-ScriptMessage @MailArguments + + $Result.MailType | Should -Be 'Group' + $Result.Status | Should -BeTrue + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly -ParameterFilter { + (@($BodyParameter.Message.ToRecipients | ForEach-Object { $_.EmailAddress.Address }) -join ',') -eq 'a@example.org,b@example.org' -and + (@($BodyParameter.Message.CcRecipients | ForEach-Object { $_.EmailAddress.Address }) -join ',') -eq 'c@example.org' -and + (@($BodyParameter.Message.BccRecipients | ForEach-Object { $_.EmailAddress.Address }) -join ',') -eq 'd@example.org' + } + } + + # 'OneOnOne' is the first MailType enum member, so its numeric value is 0. A truthiness test on the + # parameter cannot tell it apart from an unsupplied value. + It 'an explicit -MailType OneOnOne wins over the configuration' { + $Result = Send-ScriptMessage @MailArguments -MailType OneOnOne + + $Result.MailType | Should -Be 'OneOnOne' + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 4 -Exactly + } + } + + Context 'A configuration that sets MailType to OneOnOne' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + MailType = 'OneOnOne' + MgPermissionType = 'Application' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'sends each recipient a message with only them in To' { + $Result = Send-ScriptMessage @MailArguments + + $Result.MailType | Should -Be 'OneOnOne' + $Result.Status | Should -BeTrue + $Result.Error | Should -BeNullOrEmpty + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 4 -Exactly + foreach ($Address in @('a@example.org', 'b@example.org', 'c@example.org', 'd@example.org')) + { + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly -ParameterFilter { + (@($BodyParameter.Message.ToRecipients | ForEach-Object { $_.EmailAddress.Address }) -join ',') -eq $Address -and + ($null -eq $BodyParameter.Message.CcRecipients) -and + ($null -eq $BodyParameter.Message.BccRecipients) -and + ($BodyParameter.Message.Subject -eq 'Test subject') + } + } + } + + It 'sends an address listed more than once, in any case, one message' { + $Arguments = $MailArguments.Clone() + $Arguments['To'] = @('a@example.org', 'A@Example.org', 'b@example.org') + $Arguments['CC'] = 'a@example.org' + $Arguments.Remove('BCC') + + $null = Send-ScriptMessage @Arguments + + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 2 -Exactly + } + + It 'an explicit -MailType Group wins over the configuration' { + $Result = Send-ScriptMessage @MailArguments -MailType Group + + $Result.MailType | Should -Be 'Group' + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly + } + + It 'still sends to the other recipients when one fails, and names the one that failed' { + Mock -ModuleName ScriptMessage Send-MgUserMail { throw 'Mailbox unavailable' } -ParameterFilter { + $BodyParameter.Message.ToRecipients[0].EmailAddress.Address -eq 'b@example.org' + } + + $Result = Send-ScriptMessage @MailArguments + + $Result.Status | Should -Not -BeTrue + @($Result.Error).Count | Should -Be 1 + $Result.Error[0].Type | Should -Be 'Error' + $Result.Error[0].Message | Should -BeLike "*'b@example.org'*Mailbox unavailable*" + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 4 -Exactly + } + } + + Context 'A configuration with no MailType setting' { + BeforeAll { + # A configuration file can leave MailType out entirely. + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + MgPermissionType = 'Application' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'sends one message to every recipient' { + $Result = Send-ScriptMessage @MailArguments + + $Result.MailType | Should -Be 'Group' + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly + } + } + + Context 'A configuration that leaves MailType blank' { + BeforeAll { + # Templates/config_scriptmessage.json writes a setting it leaves unset as an empty string. + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail') + MailType = '' + MgPermissionType = 'Application' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + } + + It 'sends one message to every recipient' { + $Result = Send-ScriptMessage @MailArguments + + $Result.MailType | Should -Be 'Group' + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly + } + } + + Context 'A configuration with an unrecognized MailType' { + BeforeAll { + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { + $Config = [pscustomobject]@{ + MicrosoftGraph = [pscustomobject]@{ + AllowableMessageTypes = @('Mail', 'Chat') + MailType = 'Individual' + ChatType = 'OneOnOne' + IncludeBCCInGroupChat = $false + MgPermissionType = 'Delegated' + MgDisconnectWhenDone = $false + } + } + if ($null -ne $Service) { $Config.$Service } else { $Config } + } + Mock -ModuleName ScriptMessage Get-ScriptMessageContext { + [pscustomobject]@{ + Account = 'sender@example.org' + Scopes = @('Chat.ReadWrite') + } + } + Mock -ModuleName ScriptMessage New-MgChat { [pscustomobject]@{ Id = 'new-chat-id' } } + Mock -ModuleName ScriptMessage New-MgChatMessage { [pscustomobject]@{ Id = 'new-message-id' } } + } + + It 'stops a Mail send before connecting, naming the setting' { + { Send-ScriptMessage @MailArguments } | + Should -Throw "*'MailType' setting*must be one of: OneOnOne, Group*'Individual'*" + Should -Invoke -ModuleName ScriptMessage Connect-ScriptMessage -Times 0 -Exactly + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 0 -Exactly + } + + # MailType only affects Mail, so a value only Mail would read must not stop a Chat send. + It 'does not stop a Chat send' { + $Arguments = $MailArguments.Clone() + $Arguments['Type'] = 'Chat' + + $Result = Send-ScriptMessage @Arguments -ErrorAction Stop + + $Result.Error | Should -BeNullOrEmpty + # A one-on-one chat goes to every recipient, BCC included. + Should -Invoke -ModuleName ScriptMessage New-MgChatMessage -Times 4 -Exactly + } + } +} + Describe 'Send-ScriptMessage configuration defaults' { BeforeAll { Mock -ModuleName ScriptMessage Connect-ScriptMessage { } @@ -483,7 +696,7 @@ Describe 'Send-ScriptMessage configuration defaults' { } } - Context 'Boolean settings and parameters reject strings' { + Context 'Boolean settings and parameters' { BeforeAll { Mock -ModuleName ScriptMessage Disconnect-MgGraph { } @@ -534,14 +747,17 @@ Describe 'Send-ScriptMessage configuration defaults' { } } - Context 'A configuration file that quotes IncludeBCCInGroupChat' { + # A boolean setting is an opt-in flag, on only when it equals $true. Quoted text is not $true, so a + # quoted "false" must read as off; a [bool] cast would read it as on. + Context 'A configuration file that quotes IncludeBCCInGroupChat as "false"' { BeforeAll { Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { $Config = [pscustomobject]@{ MicrosoftGraph = [pscustomobject]@{ - AllowableMessageTypes = @('Mail') + AllowableMessageTypes = @('Mail', 'Chat') + ChatType = 'Group' IncludeBCCInGroupChat = 'false' - MgPermissionType = 'Application' + MgPermissionType = 'Delegated' MgDisconnectWhenDone = $false } } @@ -549,14 +765,16 @@ Describe 'Send-ScriptMessage configuration defaults' { } } - It 'stops without sending' { - { Send-ScriptMessage @MailArguments } | - Should -Throw "*'IncludeBCCInGroupChat' setting*must be a boolean*" - Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 0 -Exactly + It 'leaves BCC recipients out of the group chat' { + $Result = Send-ScriptMessage @ChatArguments -BCC 'blind@example.org' -WarningAction SilentlyContinue + + @($Result.Error | Where-Object { $_.Type -eq 'Warning' -and $_.Message -like '*not included in the group chat*' }).Count | + Should -Be 1 + Should -Invoke -ModuleName ScriptMessage New-MgChatMessage -Times 1 -Exactly } } - Context 'A configuration file that quotes MgDisconnectWhenDone' { + Context 'A configuration file that quotes MgDisconnectWhenDone as "false"' { BeforeAll { Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { $Config = [pscustomobject]@{ @@ -571,12 +789,11 @@ Describe 'Send-ScriptMessage configuration defaults' { } } - # The Microsoft Graph service consumes this setting after its message type loop, so it checks the - # value before that loop to keep an unreadable one from surfacing after the message has gone out. - It 'stops without sending' { - { Send-ScriptMessage @MailArguments } | - Should -Throw "*'MgDisconnectWhenDone' setting*must be a boolean*" - Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 0 -Exactly + It 'sends and stays connected' { + $null = Send-ScriptMessage @MailArguments + + Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly + Should -Invoke -ModuleName ScriptMessage Disconnect-MgGraph -Times 0 -Exactly } } } diff --git a/Tests/ServiceDispatch.Tests.ps1 b/Tests/ServiceDispatch.Tests.ps1 index 53ddf29..6606cda 100644 --- a/Tests/ServiceDispatch.Tests.ps1 +++ b/Tests/ServiceDispatch.Tests.ps1 @@ -37,13 +37,11 @@ Describe 'The service table' { } } - It 'records which generic settings Microsoft Graph honors, and which it does not' { + It 'records that Microsoft Graph honors every generic setting' { InModuleScope ScriptMessage { $Supported = (Get-ScriptMessageServiceRegistration -Service 'MicrosoftGraph').SupportedSetting - $Supported | Should -Contain 'ChatType' - $Supported | Should -Contain 'IncludeBCCInGroupChat' - $Supported | Should -Not -Contain 'MailType' + ($Supported | Sort-Object) -join ', ' | Should -Be 'ChatType, IncludeBCCInGroupChat, MailType' } } } @@ -55,6 +53,15 @@ Describe 'A generic setting the service does not support' { function script:Send-MgUserMail { param($UserId, $BodyParameter, [switch]$PassThru) throw 'Send-MgUserMail stand-in called without a mock.' } } + # Microsoft Graph honors every generic setting, so narrow its entry to leave one out. A service cannot + # be registered under a new name while -Service is the MessagingService enum. + InModuleScope ScriptMessage { + $script:OriginalRegistration = $script:ScriptMessageServiceTable['MicrosoftGraph'] + $Narrowed = $script:OriginalRegistration.PSObject.Copy() + $Narrowed.SupportedSetting = @('ChatType', 'IncludeBCCInGroupChat') + $script:ScriptMessageServiceTable['MicrosoftGraph'] = $Narrowed + } + Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { $Config = [pscustomobject]@{ MicrosoftGraph = [pscustomobject]@{ @@ -80,6 +87,13 @@ Describe 'A generic setting the service does not support' { } } + AfterAll { + # Leaving the narrowed entry in the table would break every later test that expects the real service. + InModuleScope ScriptMessage { + $script:ScriptMessageServiceTable['MicrosoftGraph'] = $script:OriginalRegistration + } + } + It 'reports -MailType as unsupported and still sends' { $Warnings = @() $null = Send-ScriptMessage @MailArguments -MailType Group -WarningVariable Warnings -WarningAction SilentlyContinue From 2e1e5bd4d25bf6d14422b2f07d8a0226eae036d7 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:40:18 -0500 Subject: [PATCH 10/10] Remove the SupportedSetting declaration from the service table - Register-ScriptMessageService's -SupportedSetting listed the generic settings a service honors, and Send-ScriptMessage warned when a caller passed one the service did not list. With MailType implemented, Microsoft Graph lists all three, so no real call could reach the warning. - It controlled nothing: Send-ScriptMessage passes every message parameter to every service regardless, so a service's send function already has to declare each one. It also worked per service rather than per message type, covered 3 of about 15 parameters, and wrote only to the warning stream, where scripts that inspect results never see it. - Replace it with a rule in AGENTS.md: a service that cannot honor a setting it receives adds a Warning to that message's Error property and sends what it can, as the Graph chat path already does for sending on behalf of others. - Remove the tests that covered it and the narrowed registration they needed. No changelog entry: the declaration and its warning never shipped. --- AGENTS.md | 5 ++ .../Private/Register-ScriptMessageService.ps1 | 9 --- ScriptMessage/Public/Send-ScriptMessage.ps1 | 21 ------ ScriptMessage/Services/MicrosoftGraph.ps1 | 1 - Tests/ServiceDispatch.Tests.ps1 | 74 +------------------ 5 files changed, 6 insertions(+), 104 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 75927d7..e5e66a0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -209,6 +209,11 @@ $Text = [System.IO.File]::ReadAllText($Path) `$ServiceConfig. -eq $true`, with the setting on the left, so missing, `false`, quoted text, and typos all mean off. Never read one with a `[bool]` cast, a truthiness test, or `$true -eq`: each of those reads the text `"false"` as true. Name a new boolean setting so that `true` opts in and off is the safe state. +- **A service that cannot honor a setting or parameter it receives says so in its result and sends what it + can.** Add a `Warning` entry to that message's `Error` property, as the Microsoft Graph service does for a + chat sent on behalf of someone else, rather than failing the whole call or dropping the value silently. + `Send-ScriptMessage` passes every message parameter to every service, so each service's send function must + accept all of them. ## Testing diff --git a/ScriptMessage/Private/Register-ScriptMessageService.ps1 b/ScriptMessage/Private/Register-ScriptMessageService.ps1 index b35dfc5..ca41bc3 100644 --- a/ScriptMessage/Private/Register-ScriptMessageService.ps1 +++ b/ScriptMessage/Private/Register-ScriptMessageService.ps1 @@ -11,14 +11,6 @@ function Register-ScriptMessageService [Parameter(Mandatory = $true)] [string]$Name, - # The generic, unprefixed configuration settings and parameters this service honors. Mandatory so that - # every service has to state its answer: a caller who supplies one a service does not honor is told, - # rather than having the value dropped without a word. A service's own prefixed settings are its - # business and are not listed here. - [Parameter(Mandatory = $true)] - [AllowEmptyCollection()] - [string[]]$SupportedSetting, - [Parameter(Mandatory = $true)] [string]$ConnectFunction, @@ -41,7 +33,6 @@ function Register-ScriptMessageService $script:ScriptMessageServiceTable[$Name] = [PSCustomObject]@{ Name = $Name - SupportedSetting = $SupportedSetting ConnectFunction = $ConnectFunction DisconnectFunction = $DisconnectFunction SendFunction = $SendFunction diff --git a/ScriptMessage/Public/Send-ScriptMessage.ps1 b/ScriptMessage/Public/Send-ScriptMessage.ps1 index fa28a77..c48096f 100644 --- a/ScriptMessage/Public/Send-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Send-ScriptMessage.ps1 @@ -300,27 +300,6 @@ function Send-ScriptMessage throw "The ScriptMessage configuration file has no `'$($Registration.Name)`' section. Add one for that messaging service, or send from a service the file configures." } - # Tell the caller about any generic setting this service does not honor. Services are meant to be - # interchangeable, so one that cannot act on a setting is an ordinary case rather than an error: the - # rest of the send goes ahead, which matters most when a single call goes to several services and only - # some of them honor it. - foreach ($settingName in @('MailType', 'ChatType', 'IncludeBCCInGroupChat')) - { - if (-not $PSBoundParameters.ContainsKey($settingName)) - { - continue - } - # A $null means "use the configured value", so the caller has expressed no preference to drop. - if ($null -eq $PSBoundParameters[$settingName]) - { - continue - } - if ($Registration.SupportedSetting -notcontains $settingName) - { - Write-Warning -Message "The `'$($Registration.Name)`' messaging service does not support the `'$settingName`' parameter, so it was ignored." - } - } - # Set the connection parameters. $ConnectionParameters = @{ ServiceConfig = $ServiceConfig diff --git a/ScriptMessage/Services/MicrosoftGraph.ps1 b/ScriptMessage/Services/MicrosoftGraph.ps1 index 18324f5..00908ea 100644 --- a/ScriptMessage/Services/MicrosoftGraph.ps1 +++ b/ScriptMessage/Services/MicrosoftGraph.ps1 @@ -1138,7 +1138,6 @@ function Send-ScriptMessage_MicrosoftGraph # Announce this service to the module. Keep this at the bottom of the file, after the functions it names. Register-ScriptMessageService -Name 'MicrosoftGraph' ` - -SupportedSetting 'MailType', 'ChatType', 'IncludeBCCInGroupChat' ` -ConnectFunction 'Connect-ScriptMessage_MicrosoftGraph' ` -DisconnectFunction 'Disconnect-ScriptMessage_MicrosoftGraph' ` -SendFunction 'Send-ScriptMessage_MicrosoftGraph' ` diff --git a/Tests/ServiceDispatch.Tests.ps1 b/Tests/ServiceDispatch.Tests.ps1 index 6606cda..b45cda5 100644 --- a/Tests/ServiceDispatch.Tests.ps1 +++ b/Tests/ServiceDispatch.Tests.ps1 @@ -31,83 +31,11 @@ Describe 'The service table' { It 'refuses to register a service that is not a MessagingService member' { InModuleScope ScriptMessage { - { Register-ScriptMessageService -Name 'Carrier Pigeon' -SupportedSetting @() ` + { Register-ScriptMessageService -Name 'Carrier Pigeon' ` -ConnectFunction 'c' -DisconnectFunction 'd' -SendFunction 's' -GetContextFunction 'g' } | Should -Throw '*not a member of the MessagingService enum*' } } - - It 'records that Microsoft Graph honors every generic setting' { - InModuleScope ScriptMessage { - $Supported = (Get-ScriptMessageServiceRegistration -Service 'MicrosoftGraph').SupportedSetting - - ($Supported | Sort-Object) -join ', ' | Should -Be 'ChatType, IncludeBCCInGroupChat, MailType' - } - } -} - -Describe 'A generic setting the service does not support' { - BeforeAll { - # Pester can only mock a command that exists, so the stand-in has to come before the mock. - InModuleScope ScriptMessage { - function script:Send-MgUserMail { param($UserId, $BodyParameter, [switch]$PassThru) throw 'Send-MgUserMail stand-in called without a mock.' } - } - - # Microsoft Graph honors every generic setting, so narrow its entry to leave one out. A service cannot - # be registered under a new name while -Service is the MessagingService enum. - InModuleScope ScriptMessage { - $script:OriginalRegistration = $script:ScriptMessageServiceTable['MicrosoftGraph'] - $Narrowed = $script:OriginalRegistration.PSObject.Copy() - $Narrowed.SupportedSetting = @('ChatType', 'IncludeBCCInGroupChat') - $script:ScriptMessageServiceTable['MicrosoftGraph'] = $Narrowed - } - - Mock -ModuleName ScriptMessage Get-ScriptMessageConfig { - $Config = [pscustomobject]@{ - MicrosoftGraph = [pscustomobject]@{ - AllowableMessageTypes = @('Mail') - ChatType = 'Group' - IncludeBCCInGroupChat = $false - MgPermissionType = 'Application' - MgDisconnectWhenDone = $false - } - } - if ($null -ne $Service) { $Config.$Service } else { $Config } - } - Mock -ModuleName ScriptMessage Connect-ScriptMessage { } - Mock -ModuleName ScriptMessage Send-MgUserMail { $true } - - $MailArguments = @{ - Service = 'MicrosoftGraph' - Type = 'Mail' - From = 'sender@example.org' - To = 'recipient@example.org' - Subject = 'Test subject' - Body = 'Test body' - } - } - - AfterAll { - # Leaving the narrowed entry in the table would break every later test that expects the real service. - InModuleScope ScriptMessage { - $script:ScriptMessageServiceTable['MicrosoftGraph'] = $script:OriginalRegistration - } - } - - It 'reports -MailType as unsupported and still sends' { - $Warnings = @() - $null = Send-ScriptMessage @MailArguments -MailType Group -WarningVariable Warnings -WarningAction SilentlyContinue - - ($Warnings | Out-String) | Should -BeLike "*does not support the 'MailType' parameter*" - Should -Invoke -ModuleName ScriptMessage Send-MgUserMail -Times 1 -Exactly - } - - It 'says nothing about a setting the service does support' { - $Warnings = @() - $null = Send-ScriptMessage @MailArguments -ChatType Group -WarningVariable Warnings -WarningAction SilentlyContinue - - ($Warnings | Out-String) | Should -Not -BeLike '*does not support*' - } } Describe 'The public cmdlets dispatch through the service table' {