diff --git a/AGENTS.md b/AGENTS.md index e5e66a0..61f030d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -206,8 +206,8 @@ $Text = [System.IO.File]::ReadAllText($Path) - **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 + `$ServiceConfig. -eq $true`, with the setting on the left, so missing, `false`, typos, and any quoted + text except `"true"` (in any letter case, which counts as on) 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index bee4a67..ac1b678 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,9 +14,14 @@ - `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: Improved how `Send-ScriptMessage` validates configuration file settings, so a value in a form a setting does not support is no longer misread. +- Minor: `Send-ScriptMessage` now reads every true/false setting in the configuration file the same way, so a value other than `true` no longer turns a setting on. - 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`. +- Minor: `Get-ScriptMessageConfig`, `Connect-ScriptMessage`, and `Send-ScriptMessage` now say why a configuration file could not be used. + +### Security + +- `MgApp_CertificatePath` ran any PowerShell code written into it, so anyone who could edit the configuration file could run commands as the account sending messages. This affected every configuration file on PowerShell 7.4 and later, whatever authentication it was set up for. Only environment variables written as `$env:NAME` or `${env:NAME}` are expanded now, and the rest of the path is used exactly as written. --- ## [1.1.1](https://github.com/Sekers/ScriptMessage/tree/1.1.1) - 2026-09-16 diff --git a/Research_Notes/PowerShell-Language-Behavior.md b/Research_Notes/PowerShell-Language-Behavior.md index 92891b7..44b223e 100644 --- a/Research_Notes/PowerShell-Language-Behavior.md +++ b/Research_Notes/PowerShell-Language-Behavior.md @@ -87,8 +87,42 @@ without renaming the property. - `[Environment]::ExpandEnvironmentVariables('%SystemRoot%\x')` returned `C:\Windows\x`. It only substitutes environment variables. -**From source:** `Connect-ScriptMessage_MicrosoftGraph` passes the `MgApp_CertificatePath` configuration value -through `ExpandString`, so that setting can execute code written into the configuration file. +What `ExpandString` can reach when a module function calls it was measured on **2026-09-21** with PowerShell +**7.6.6** only, using a stand-in module laid out like this one: a `.psm1` that dot-sources a function from a +`Services\` file, and that function passing its argument to `ExpandString`. The calling script lived in a +different folder, set a script variable, and set a global one. + +| Text expanded | Calling script run with `pwsh -File` | Calling script run with `&` | +| --- | --- | --- | +| `$PSScriptRoot\cert.pfx` | the module's `Services` folder | the same | +| a variable the calling script set | its value | empty | +| a global variable | its value | its value | +| `$env:SystemRoot\cert.pfx`, `${env:ProgramFiles(x86)}\cert.pfx` | expanded | expanded | +| `$HOME\cert.pfx` | expanded | expanded | +| `C:\certs\$app\cert.pfx` | `C:\certs\\cert.pfx` | the same | +| ``C:\certs\a`b.pfx`` | `C:\certs\a.pfx` | the same | +| `\\server\c$\certs\cert.pfx` | unchanged | the same | + +`$PSScriptRoot` inside a function is the folder of the file that defines the function, never the caller's. The +same day, the real `Connect-ScriptMessage_MicrosoftGraph`, with `Get-PfxCertificate` and `Connect-MgGraph` +replaced by stand-ins inside the module, passed `...\ScriptMessage\Services\Config\PrivateKeyCertificate.pfx` to +`Get-PfxCertificate` for the setting `$PSScriptRoot\Config\PrivateKeyCertificate.pfx`. + +**From source:** every release from `1.0.0` to `1.1.1` passed `MgApp_CertificatePath` through `ExpandString` in a +function defined in the module's `Services` folder (`MgGraph.ps1`, then `MicrosoftGraph.ps1`), so that setting +ran any code written into it, and `$PSScriptRoot` in it meant the module's `Services` folder. +`Connect-ScriptMessage_MicrosoftGraph` now expands only `$env:NAME` and `${env:NAME}` in that setting, with a +`-replace` that looks each name up with `[Environment]::GetEnvironmentVariable`, and uses the rest as written. A +scriptblock replacement needs PowerShell 6 or later, which is safe there because certificate file +authentication throws before PowerShell 7.4. + +### What this does not establish + +- The scope table comes from the stand-in module. Only `$PSScriptRoot` was checked against the real function. +- Windows PowerShell 5.1 was not run. The setting is unused there, since certificate file authentication needs + PowerShell 7.4 or later. +- **Unverified:** why a calling script's variables are visible under `pwsh -File`. The results fit `-File` + running the script in the global scope, but that is inferred, not traced. ## 5. Measured: looking up a property by a name held in an array @@ -307,8 +341,9 @@ three states it needs (unset, true, false) and makes the binder reject a string. 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. +`Connect-ScriptMessage_MicrosoftGraph`. A missing setting, `false`, a typo, and quoted text all read as off, which +is the safe state of all four settings, except that quoted `"true"` in any letter case reads as on, as the table +above shows. Unquoted JSON `true` and `false` arrive as real booleans. ### What this does not establish @@ -321,3 +356,53 @@ is the safe state of all four. Unquoted JSON `true` and `false` arrive as real b - 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. + +## 11. Measured: catching file errors, and `ConvertFrom-Json` messages that repeat their input + +Measured on **2026-09-21**, same machine and editions as the header, in a standalone script with the module not +imported. The editions differ on `ConvertFrom-Json`, as the second table shows. + +A .NET method called from PowerShell throws its exception wrapped in a `MethodInvocationException`. A typed +`catch []` still matches the exception inside, and there `$_.Exception` is that inner exception. A plain +`catch` receives the wrapper, and `$_.Exception.GetBaseException()` returns the inner one. Each path below went +through `$PSCmdlet.GetUnresolvedProviderPathFromPSPath()` and then `[System.IO.StreamReader]::new()`: + +| Path | Exception | Matched by a typed `catch` for it | +| --- | --- | --- | +| a file that does not exist | `System.IO.FileNotFoundException`: "Could not find file '...'" with the full path | yes | +| a file in a folder that does not exist | `System.IO.DirectoryNotFoundException`: "Could not find a part of the path '...'" | yes | +| a drive that does not exist, `Q:\missing.json` | `System.Management.Automation.DriveNotFoundException`, from the path resolution | yes | +| a folder | `System.UnauthorizedAccessException`: "Access to the path '...' is denied." | not tried; a plain `catch` got it | + +`ConvertFrom-Json -ErrorAction Stop` throws `System.ArgumentException` in both editions. Windows PowerShell 5.1 +ends many of its messages with a position in parentheses, such as `(45):`, followed by the entire text it was +given: + +| Input | Windows PowerShell 5.1 | PowerShell 7 | +| --- | --- | --- | +| a missing colon, `"MgClientID" "client-id"` | `Invalid object passed in, ':' or '}' expected. (45):` then the input | `Invalid character after parsing property name. Expected ':' but got: ". Path 'MgTenantID', line 1, position 44.` | +| a missing closing `}` | `Invalid object passed in, ':' or '}' expected. (83):` then the input | `Unexpected end when deserializing object. Path 'Secret', line 1, position 83.` | +| a single backslash, `"C:\Certs\a.pfx"` | `Unrecognized escape sequence. (44):` then the input | `Bad JSON escape sequence: \C. Path 'Path', line 1, position 44.` | +| a missing closing `]` | `Invalid array passed in, ',' expected. (32):` then the input | no error | +| `not json` | `Invalid JSON primitive: not.` | `Unexpected character encountered while parsing value: n. Path '', line 0, position 0.` | +| a missing value, `"Bad": ,`, or a trailing comma | `Invalid JSON primitive: .` | no error | +| an empty string | no error; the result is `$null` | the same | + +Every PowerShell 7 message starts with `Conversion from JSON failed with error:`, omitted above. A 5,076-character +input with a missing colon near its start gave a 5,129-character message on 5.1 ending with the input's last +line, so the input is not truncated. + +**From source:** `Get-ScriptMessageConfig` catches the three not-found exceptions above by type to report a +missing file, and reports anything else from reading the file as unreadable, both with +`GetBaseException().Message`. It removes the `():` position and everything after it from a `ConvertFrom-Json` message, so +the configuration file, which holds the tenant ID, client ID, and encrypted secrets, never reaches the error. An +empty configuration file reads as `$null` without an error in both editions. + +### What this does not establish + +- Only the inputs above were tried. That every 5.1 message repeating the input uses the `():` form is + inferred from them, not read from the source of the serializer 5.1 uses. +- A message without that form can still quote one token: `Invalid JSON primitive` names the unquoted text it + stopped at, so an unquoted secret would appear in it. +- What PowerShell 7 builds from the input it accepts but 5.1 rejects was not examined. +- A file the account has no permission to read was not tested, only a folder given as the path. diff --git a/ScriptMessage/Public/Get-ScriptMessageConfig.ps1 b/ScriptMessage/Public/Get-ScriptMessageConfig.ps1 index dc85ee2..752ee16 100644 --- a/ScriptMessage/Public/Get-ScriptMessageConfig.ps1 +++ b/ScriptMessage/Public/Get-ScriptMessageConfig.ps1 @@ -45,7 +45,7 @@ function Get-ScriptMessageConfig throw "`'`$ScriptMessage_Global_ConfigFilePath`' is not specified. Don't forget to first use the `'Set-ScriptMessageConfigFilePath`' cmdlet!" } - # Get Config and Secrets + # Read the configuration file. try { # .NET resolves a relative path against the process directory, not PowerShell's current location. @@ -67,19 +67,36 @@ function Get-ScriptMessageConfig { $Reader.Dispose() } + } + catch [System.IO.FileNotFoundException], [System.IO.DirectoryNotFoundException], [System.Management.Automation.DriveNotFoundException] + { + throw "Can't find the JSON configuration file. $($_.Exception.GetBaseException().Message)" + } + catch + { + throw "Can't read the JSON configuration file. $($_.Exception.GetBaseException().Message)" + } - $ScriptMessageConfig = $ConfigText | ConvertFrom-Json - if (-not ($null -eq $Service)) - { - return $ScriptMessageConfig.$Service - } - else - { - return $ScriptMessageConfig - } + # Parse it. + try + { + $ScriptMessageConfig = $ConfigText | ConvertFrom-Json -ErrorAction Stop } catch { - throw "Can't find the JSON configuration file. Use 'Set-ScriptMessageConfigFilePath' to create one." + # Windows PowerShell 5.1 ends many of these messages with the entire text it was parsing, which here is the + # configuration file and its secrets, so keep only the reason in front of it. See section 11 of + # Research_Notes/PowerShell-Language-Behavior.md. + $Reason = $_.Exception.Message -replace '(?s) \(\d+\): .*$', '' + throw "The configuration file '$FullPath' is not valid JSON. $Reason" + } + + if (-not ($null -eq $Service)) + { + return $ScriptMessageConfig.$Service + } + else + { + return $ScriptMessageConfig } } \ No newline at end of file diff --git a/ScriptMessage/Public/Send-ScriptMessage.ps1 b/ScriptMessage/Public/Send-ScriptMessage.ps1 index c48096f..4b11f7b 100644 --- a/ScriptMessage/Public/Send-ScriptMessage.ps1 +++ b/ScriptMessage/Public/Send-ScriptMessage.ps1 @@ -321,8 +321,9 @@ function Send-ScriptMessage # 'IncludeBCCInGroupChat' accepts $null to mean "use the configured value", which is also what an # 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. + # $true. Text equals $true only when it reads "true" in any letter case, so anything else, including + # the text "false", 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 diff --git a/ScriptMessage/Services/MicrosoftGraph.ps1 b/ScriptMessage/Services/MicrosoftGraph.ps1 index 00908ea..aaeeae4 100644 --- a/ScriptMessage/Services/MicrosoftGraph.ps1 +++ b/ScriptMessage/Services/MicrosoftGraph.ps1 @@ -372,7 +372,10 @@ function Connect-ScriptMessage_MicrosoftGraph throw $NewMessage } - $MgApp_CertificatePath = $ExecutionContext.InvokeCommand.ExpandString($ServiceConfig.MgApp_CertificatePath) + # Expand only environment variables, so the configuration file cannot run code; the rest is used as written. + $MgApp_CertificatePath = $ServiceConfig.MgApp_CertificatePath -replace '\$\{env:([^}]+)\}|\$env:(\w+)', { + [Environment]::GetEnvironmentVariable($_.Groups[1].Value + $_.Groups[2].Value) + } # Try accessing private key certificate without password using current process credentials. [X509Certificate]$MgApp_Certificate = $null diff --git a/Tests/Connect-ScriptMessage.Tests.ps1 b/Tests/Connect-ScriptMessage.Tests.ps1 index ed608af..64e7d5a 100644 --- a/Tests/Connect-ScriptMessage.Tests.ps1 +++ b/Tests/Connect-ScriptMessage.Tests.ps1 @@ -68,3 +68,44 @@ Describe 'Connect-ScriptMessage Microsoft Graph module check' { } } } + +# Certificate file authentication throws before PowerShell 7.4, so there is no path to check there. +Describe 'Connect-ScriptMessage certificate file path' -Skip:($PSVersionTable.PSVersion -lt [version]'7.4') { + BeforeAll { + $env:SCRIPTMESSAGE_TEST_CERTS = 'C:\Certs' + + Mock -ModuleName ScriptMessage Connect-MgGraph { } + Mock -ModuleName ScriptMessage Import-Module { } + Mock -ModuleName ScriptMessage Get-PfxCertificate { } + Mock -ModuleName ScriptMessage Get-Module { + $Modules = @( + New-Module -Name 'Microsoft.Graph.Authentication' -ScriptBlock { } + New-Module -Name 'Microsoft.Graph.Teams' -ScriptBlock { } + ) + if ($Name) { $Modules | Where-Object { $_.Name -in $Name } } else { $Modules } + } + } + + AfterAll { + Remove-Item -Path 'Env:\SCRIPTMESSAGE_TEST_CERTS' -ErrorAction SilentlyContinue + } + + It 'opens when the setting is ' -ForEach @( + @{ Setting = '$env:SCRIPTMESSAGE_TEST_CERTS\app.pfx'; Expected = 'C:\Certs\app.pfx' } + @{ Setting = '${env:SCRIPTMESSAGE_TEST_CERTS}\app.pfx'; Expected = 'C:\Certs\app.pfx' } + @{ Setting = 'C:\Certs\$([string]::Concat(''a'', ''b'')).pfx'; Expected = 'C:\Certs\$([string]::Concat(''a'', ''b'')).pfx' } + @{ Setting = 'C:\Certs\$app\app.pfx'; Expected = 'C:\Certs\$app\app.pfx' } + ) { + $ServiceConfig = [pscustomobject]@{ + AllowableMessageTypes = @('Chat') + MgPermissionType = 'Application' + MgTenantID = 'tenant-id' + MgClientID = 'client-id' + MgApp_AuthenticationType = 'CertificateFile' + MgApp_CertificatePath = $Setting + } + + Connect-ScriptMessage -Service MicrosoftGraph -ServiceConfig $ServiceConfig -ErrorAction Stop + Should -Invoke -ModuleName ScriptMessage Get-PfxCertificate -Times 1 -Exactly -ParameterFilter { $FilePath -eq $Expected } + } +} diff --git a/Tests/Get-ScriptMessageConfig.Tests.ps1 b/Tests/Get-ScriptMessageConfig.Tests.ps1 index 44da781..e6cea73 100644 --- a/Tests/Get-ScriptMessageConfig.Tests.ps1 +++ b/Tests/Get-ScriptMessageConfig.Tests.ps1 @@ -104,3 +104,23 @@ Describe 'Get-ScriptMessageConfig paths' { { Get-ScriptMessageConfig -Path (Join-Path $TestDrive 'missing.json') } | Should -Throw -ExpectedMessage "Can't find the JSON configuration file*" } } + +Describe 'Get-ScriptMessageConfig errors' { + It 'reports a folder that does not exist' { + { Get-ScriptMessageConfig -Path (Join-Path $TestDrive 'No Such Folder\config.json') } | + Should -Throw -ExpectedMessage "Can't find the JSON configuration file*No Such Folder*" + } + + It 'reports a path it cannot read as a file' { + { Get-ScriptMessageConfig -Path $TestDrive } | Should -Throw -ExpectedMessage "Can't read the JSON configuration file*" + } + + # Windows PowerShell 5.1 ends its message for this mistake with the whole file, secrets included. + It 'reports a file that is not valid JSON, without repeating its contents' { + $Path = Join-Path $TestDrive 'invalid.json' + [System.IO.File]::WriteAllText($Path, '{ "MicrosoftGraph": { "MgClientID" "not-a-real-secret" } }') + + $ErrorRecord = { Get-ScriptMessageConfig -Path $Path } | Should -Throw -ExpectedMessage "The configuration file '*invalid.json' is not valid JSON.*" -PassThru + $ErrorRecord.Exception.Message | Should -Not -BeLike '*not-a-real-secret*' + } +} diff --git a/Tests/MgGraphDynamicParams.ps1 b/Tests/MgGraphDynamicParams.ps1 deleted file mode 100644 index 2b634ad..0000000 --- a/Tests/MgGraphDynamicParams.ps1 +++ /dev/null @@ -1,52 +0,0 @@ -DynamicParam -{ - # Initialize Parameter Dictionary - $ParameterDictionary = [System.Management.Automation.RuntimeDefinedParameterDictionary]::new() - - # Make Mg* parameters appear only if messaging 'Service' is 'MicrosoftGraph'. - if ($Service -eq 'MicrosoftGraph') - { - $ParameterAttributes = [System.Management.Automation.ParameterAttribute]@{ - ParameterSetName = "MicrosoftGraph" - Mandatory = $true - ValueFromPipeline = $true - ValueFromPipelineByPropertyName = $true - } - - $AttributeCollection = [System.Collections.ObjectModel.Collection[System.Attribute]]::new() - $AttributeCollection.Add($ParameterAttributes) - - $DynamicParameter1 = [System.Management.Automation.RuntimeDefinedParameter]::new( - 'MgPermissionType', [string], $AttributeCollection) # Delegated or Application. See: https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-permissions-and-consent#permission-types and https://docs.microsoft.com/en-us/graph/auth/auth-concepts#delegated-and-application-permissions. - - $DynamicParameter2 = [System.Management.Automation.RuntimeDefinedParameter]::new( - 'MgDisconnectWhenDone', [bool], $AttributeCollection) - - $DynamicParameter3 = [System.Management.Automation.RuntimeDefinedParameter]::new( - 'MgTenantID', [string], $AttributeCollection) - - $DynamicParameter4 = [System.Management.Automation.RuntimeDefinedParameter]::new( - 'MgClientID', [string], $AttributeCollection) - - $ParameterDictionary.Add('MgPermissionType', $DynamicParameter1) - $ParameterDictionary.Add('MgDisconnectWhenDone', $DynamicParameter2) - $ParameterDictionary.Add('MgTenantID', $DynamicParameter3) - $ParameterDictionary.Add('MgClientID', $DynamicParameter4) - } - - return $ParameterDictionary -} - -begin -{ - # Set Variables From Dynamic Parameters - switch ($Service) - { - 'MicrosoftGraph' { - $MgPermissionType = $PSBoundParameters['MgPermissionType'] - $MgDisconnectWhenDone = $PSBoundParameters['MgDisconnectWhenDone'] - $MgTenantID = $PSBoundParameters['MgTenantID'] - $MgClientID = $PSBoundParameters['MgClientID'] - } - } -} diff --git a/Tests/ScriptMessage_Testing (Installed Module).ps1 b/Tests/ScriptMessage_Testing (Installed Module).ps1 index da82efd..2f65614 100644 --- a/Tests/ScriptMessage_Testing (Installed Module).ps1 +++ b/Tests/ScriptMessage_Testing (Installed Module).ps1 @@ -19,7 +19,7 @@ $ErrorActionPreference = 'Stop' ############################## # Import ScriptMessage Module -Import-Module Script-Message +Import-Module ScriptMessage # Set ScriptMessage MessagingConfiguration Path Set-ScriptMessageConfigFilePath -Path "$PSScriptRoot\Config\config_scriptmessage.json" diff --git a/Tests/Send-ScriptMessage.Tests.ps1 b/Tests/Send-ScriptMessage.Tests.ps1 index bc94774..7a8b0bb 100644 --- a/Tests/Send-ScriptMessage.Tests.ps1 +++ b/Tests/Send-ScriptMessage.Tests.ps1 @@ -747,8 +747,9 @@ Describe 'Send-ScriptMessage configuration defaults' { } } - # 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. + # A boolean setting is an opt-in flag, on only when it equals $true. Text equals $true only when it + # reads "true" in any letter case, 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 {