fix(privileges): report a malformed EnablePrivileges setting with one clear error

BaseCmdletWithPrivControl.BeginProcessing and EndProcessing read
(bool)privateData["EnablePrivileges"] without checking it first; a
setting that is not a Boolean, such as the string '$true' from a typo
in the module manifest, or a missing key, made every cmdlet derived
from the class fail with an InvalidCastException or a
NullReferenceException before anything useful was said.
Enable-Privileges had its own try/catch for the same cast that raised a
different message (a ParseException), but it was unreachable, because
BeginProcessing always failed first.

A new helper, GetEnablePrivilegesSetting, centralizes the check in the
base class: a setting that is missing or not a Boolean now stops the
cmdlet with one terminating error, id EnablePrivilegesSettingInvalid,
category InvalidData, that names the setting, says it must be $true or
$false, and names the module manifest. BeginProcessing, EndProcessing,
and Enable-Privileges all use it now; the dead try/catch of
Enable-Privileges is removed, so a mistaken setting no longer produces
two different messages depending on which cmdlet hit it first. The
valid settings $true and $false keep today's behavior.

GetInheritedFrom (used by Get-NTFSAccess and Get-NTFSAudit),
GetFileSystemModeProperty, and IdentifyHardLinks (both used by
Get-ChildItem2) read their own setting the same unchecked way and share
this weakness; they are not changed here; see the final report.

Breaking: a script that caught the old InvalidCastException,
NullReferenceException, or ParseException for a malformed
EnablePrivileges setting now needs to catch the terminating error with
id EnablePrivilegesSettingInvalid instead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>

Docs/Concepts.md

@@ -274,6 +274,12 @@ change the module's behavior:
 cost extra work for every item. Turn them off to speed up large folder
 trees when you don't need the information.
 
+A malformed `EnablePrivileges` setting, such as a string instead of
+`$true` or `$false`, or a missing key, stops every cmdlet that uses
+privileges, and `Enable-Privileges`, with the error
+`EnablePrivilegesSettingInvalid` naming the setting, instead of an
+unrelated exception.
+
 To change a setting for the current session only, change the value after
 you import the module:
 

NTFSSecurity/BaseCmdlets.cs

@@ -354,7 +354,6 @@ namespace NTFSSecurity
         protected PrivilegeAndAttributesCollection privileges = null;
         protected PrivilegeControl privControl = new PrivilegeControl();
         private List<string> enabledPrivileges = new List<string>();
-        Hashtable privateData = null;
 
         // A cmdlet that enables the privileges for the session, such as Enable-Privileges, keeps them enabled.
         protected virtual bool KeepEnabledPrivileges
@@ -364,9 +363,7 @@ namespace NTFSSecurity
 
         protected override void BeginProcessing()
         {
-            privateData = (Hashtable)MyInvocation.MyCommand.Module.PrivateData;
-
-            if ((bool)privateData["EnablePrivileges"])
+            if (GetEnablePrivilegesSetting())
             {
                 WriteVerbose("EnablePrivileges enabled in PrivateDate");
                 EnableFileSystemPrivileges(true);
@@ -375,7 +372,7 @@ namespace NTFSSecurity
 
         protected override void EndProcessing()
         {
-            if ((bool)privateData["EnablePrivileges"])
+            if (GetEnablePrivilegesSetting())
             {
                 WriteVerbose("EnablePrivileges enabled in PrivateDate");
 
@@ -395,6 +392,34 @@ namespace NTFSSecurity
             }
         }
 
+        /// <summary>
+        /// Reads the EnablePrivileges setting of the module manifest (NTFSSecurity.psd1, section PrivateData) and
+        /// terminates with one EnablePrivilegesSettingInvalid error when the setting is missing or is not a Boolean.
+        /// Before 5.0.0, BeginProcessing, EndProcessing, and Enable-Privileges each cast the setting on their own
+        /// without checking it first, so a missing key or a value that was not a Boolean, such as the string '$true'
+        /// from a typo in the manifest, failed with a NullReferenceException or an InvalidCastException before
+        /// anything useful was said, and Enable-Privileges raised a second, different message for the same cause
+        /// that was never reached, because BeginProcessing always failed first.
+        /// </summary>
+        protected bool GetEnablePrivilegesSetting()
+        {
+            var moduleData = (Hashtable)MyInvocation.MyCommand.Module.PrivateData;
+            var value = moduleData == null ? null : moduleData["EnablePrivileges"];
+
+            if (!(value is bool))
+            {
+                var found = value == null ? "missing" : string.Format("'{0}'", value);
+                var message = string.Format(
+                    "The 'EnablePrivileges' setting in the PrivateData section of the module manifest (NTFSSecurity.psd1) " +
+                    "must be $true or $false, but it is {0}. See the NTFSSecurity documentation for EnablePrivileges.",
+                    found);
+
+                ThrowTerminatingError(new ErrorRecord(new ArgumentException(message), "EnablePrivilegesSettingInvalid", ErrorCategory.InvalidData, value));
+            }
+
+            return (bool)value;
+        }
+
         // PowerShell calls Dispose also when a later command or a terminating error stops the pipeline, and then
         // skips EndProcessing. Before 5.0.0-rc6, the privileges that the cmdlet had enabled stayed enabled in the
         // session in that case. PowerShell ignores an exception from Dispose and no stream is open anymore, so a

NTFSSecurity/OtherCmdlets.cs

@@ -29,17 +29,12 @@ namespace NTFSSecurity
 
         protected override void ProcessRecord()
         {
-            var privateData = (System.Collections.Hashtable)this.MyInvocation.MyCommand.Module.PrivateData;
             var psCallStack = (CallStackFrame)this.InvokeCommand.InvokeScript("Get-PSCallStack")[1].BaseObject;
 
-            try
-            {
-                enablePrivileges = (bool)privateData["EnablePrivileges"];
-            }
-            catch (Exception ex)
-            {
-                throw new ParseException("Could not parse the module's PrivateData field in the module's psd1 file. Please refer to the documentation for further details", ex);
-            }
+            // The base class raises one EnablePrivilegesSettingInvalid error for a setting that is not $true or
+            // $false; before 5.0.0, this had its own try/catch that raised a ParseException for the same cast, but
+            // it never ran, because BeginProcessing always failed first with its own uncaught cast.
+            enablePrivileges = GetEnablePrivilegesSetting();
 
             //if the command is called from NTFSSecurity.Init.ps1 and EnablePrivileges is set to true in the NTFSSecurity.psd1 or if the cmdlet is called from somewhere else
             if ((psCallStack.InvocationInfo.MyCommand.Name == "NTFSSecurity.Init.ps1" && enablePrivileges == true))

Tests/Privileges.Tests.ps1

@@ -663,3 +663,40 @@ Describe 'The PrivilegeControl class' {
         Get-BackupPrivilegeState | Should -Be 'Disabled'
     }
 }
+
+# Before 5.0.0, a module setting that was not a Boolean (a typo such as the string '$true', or a missing key) made
+# every cmdlet derived from BaseCmdletWithPrivControl fail in BeginProcessing with an InvalidCastException or a
+# NullReferenceException before anything useful was said, and Enable-Privileges had its own try/catch for the same
+# cast that never ran, because BeginProcessing always failed first. One EnablePrivilegesSettingInvalid error now
+# names the setting instead, for a cmdlet and for Enable-Privileges alike. The test needs no elevation.
+Describe 'A malformed EnablePrivileges setting' {
+    BeforeAll {
+        $file = New-TestSandboxItem -Sandbox $sandbox -Name 'MalformedSetting'
+    }
+
+    It 'Should stop a cmdlet and Enable-Privileges with one EnablePrivilegesSettingInvalid error for <Case>, not an InvalidCastException or a NullReferenceException' -ForEach @(
+        @{ Case = 'a string value'; Setting = 'maybe' }
+        @{ Case = 'a missing value'; Setting = $null }
+    ) {
+        $privateData['EnablePrivileges'] = $Setting
+
+        try {
+            $cmdletFailure = { Get-NTFSInheritance -Path $file -ErrorAction Stop } | Should -Throw -PassThru
+            $enableFailure = { Enable-Privileges -ErrorAction Stop } | Should -Throw -PassThru
+        }
+        finally {
+            $privateData['EnablePrivileges'] = $enablePrivileges
+        }
+
+        foreach ($failure in $cmdletFailure, $enableFailure) {
+            $failure.FullyQualifiedErrorId | Should -BeLike 'EnablePrivilegesSettingInvalid,*'
+            $failure.Exception.Message | Should -BeLike '*EnablePrivileges*'
+            $failure.Exception | Should -Not -BeOfType [System.InvalidCastException]
+            $failure.Exception | Should -Not -BeOfType [System.NullReferenceException]
+        }
+
+        # Restoring the setting must leave the session usable again, for this test and the ones that follow it.
+        $privateData['EnablePrivileges'] | Should -Be $enablePrivileges
+        { Get-NTFSInheritance -Path $file -ErrorAction Stop } | Should -Not -Throw
+    }
+}