Browse Source

fix(access): reject a list for -AccessType and -AppliesTo, and complete in capitals in Turkish

-AccessType and -AppliesTo of Add-NTFSAccess, Remove-NTFSAccess,
Add-NTFSAudit (-AppliesTo), and Remove-NTFSAudit (-AppliesTo) take one value,
but PowerShell adds up the numbers of a comma list or an array for an
enumeration, so that -AppliesTo 'ThisFolderOnly,FilesOnly' created a
FilesOnly entry and -AccessType 'Allow,Deny' created a Deny entry, without a
message. The conversion in ParameterConversion now rejects a string with a
comma and a collection of more than one element for an enumeration that is
not a flags enumeration, with the same InvalidParameterValue error as for a
misspelled name (terminating for a named value, one error per object for a
piped one). A single name and a collection of one element convert as before,
and the flags parameters still take a list. The valid-value matrices of both
published rc8 and this build give the same entries in both editions, named,
piped by property name, and splatted, apart from the nine list rows.

The completers of the five parameters compared with WildcardOptions.IgnoreCase
only, so that a word typed in capitals did not complete while the culture of
the thread was Turkish (an I and an i are different letters there, and
THISFOLDERO did not match ThisFolderOnly). They compare culture invariant now,
like the filter of Get-ChildItem2.

The tests pin both, red before the change and green after it in four
configurations, and the headers of the two test files no longer say that the
-SecurityDescriptor sets have no handler per descriptor.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
pull/131/head
Raimund Andree 1 week ago
parent
commit
1b8a3763a1
  1. 2
      NTFSSecurity/EnumArgumentCompleters.cs
  2. 51
      NTFSSecurity/ParameterConversion.cs
  3. 47
      Tests/ParameterSets.Access.Tests.ps1
  4. 43
      Tests/ParameterSets.Audit.Tests.ps1

2
NTFSSecurity/EnumArgumentCompleters.cs

@ -38,7 +38,7 @@ namespace NTFSSecurity
}
}
var pattern = new WildcardPattern(word + "*", WildcardOptions.IgnoreCase);
var pattern = new WildcardPattern(word + "*", WildcardOptions.IgnoreCase | WildcardOptions.CultureInvariant);
var results = new List<CompletionResult>();
foreach (var name in Enum.GetNames(enumType))

51
NTFSSecurity/ParameterConversion.cs

@ -1,4 +1,5 @@
using System;
using System.Collections;
using System.Management.Automation;
namespace NTFSSecurity
@ -16,7 +17,9 @@ namespace NTFSSecurity
/// <summary>
/// Converts the raw value of a parameter to T, the way the PowerShell binder converts an argument for a
/// parameter of that type: names in any case, a comma-separated list for a flags enumeration, a number, or
/// an enumeration instance.
/// an enumeration instance. An enumeration that is not a flags enumeration takes one value: PowerShell would
/// add up the numbers of a list for it, which selects a value that the caller did not name, so a list is
/// rejected for it.
/// </summary>
/// <param name="value">
/// The raw value of the parameter. Not null: the caller keeps the default of the parameter for null instead
@ -25,9 +28,10 @@ namespace NTFSSecurity
/// <param name="parameterName">The name of the parameter, without the leading dash, for the message.</param>
/// <returns>The converted value.</returns>
/// <exception cref="ArgumentException">
/// The value is blank (an empty or a white-space-only string, the usual blank CSV cell) or the conversion
/// failed. The message names the value, unless it is blank, the parameter, and the valid names of T; the
/// inner exception of a failed conversion is the error that LanguagePrimitives.ConvertTo raised.
/// The value is blank (an empty or a white-space-only string, the usual blank CSV cell), is a list for an
/// enumeration that is not a flags enumeration, or the conversion failed. The message names the value, unless
/// it is blank, the parameter, and the valid names of T; the inner exception of a failed conversion is the
/// error that LanguagePrimitives.ConvertTo raised.
/// </exception>
internal static T Convert<T>(object value, string parameterName)
{
@ -38,6 +42,17 @@ namespace NTFSSecurity
"The value for parameter -{0} is blank. {1}", parameterName, DescribeValidValues(typeof(T))));
}
if (!typeof(T).IsDefined(typeof(FlagsAttribute), false))
{
var list = DescribeList(value);
if (list != null)
{
throw new ArgumentException(string.Format(
"The value '{0}' for parameter -{1} is not valid: the parameter takes one value, not a list. {2}",
list, parameterName, DescribeValidValues(typeof(T))));
}
}
try
{
return (T)LanguagePrimitives.ConvertTo(value, typeof(T));
@ -63,6 +78,34 @@ namespace NTFSSecurity
return string.Format("The value for parameter -{0} is null. {1}", parameterName, DescribeValidValues(typeof(T)));
}
// The text of a list that the conversion would add up, for the message: a string that holds a comma, or a
// collection of more than one element (a wrapped PSObject counts as what it wraps). Null for any other value,
// such as a single name or a collection of one element, which convert to that one value.
private static string DescribeList(object value)
{
var unwrapped = value is PSObject ? ((PSObject)value).BaseObject : value;
var text = unwrapped as string;
if (text != null)
{
return text.IndexOf(',') >= 0 ? text : null;
}
var list = unwrapped as IList;
if (list == null || list.Count < 2)
{
return null;
}
var items = new string[list.Count];
for (var i = 0; i < items.Length; i++)
{
items[i] = string.Format("{0}", list[i]);
}
return string.Join(", ", items);
}
// The valid names of the enumeration, in declaration order, with a note for a flags enumeration that a
// combination of them is written separated with commas.
private static string DescribeValidValues(Type enumType)

47
Tests/ParameterSets.Access.Tests.ps1

@ -4,8 +4,7 @@
found untested, with the module built in NTFSSecurity\bin\Release on files in a sandbox folder. Each test closes a
gap the matrix recorded for the row it names (state, error, pipeline, continuation, or output); it does not repeat
what Access.Tests.ps1, PathErrors.Tests.ps1, PermissionScopes.Tests.ps1, SecurityDescriptorSets.Tests.ps1, or
PipelineControl.Tests.ps1 already assert. The -SecurityDescriptor sets of Add-NTFSAccess and Remove-NTFSAccess have
no per-item error handler (an open item for the maintainer); no test here makes one of those two sets fail.
PipelineControl.Tests.ps1 already assert.
#>
[Diagnostics.CodeAnalysis.SuppressMessageAttribute(
'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.'
@ -1351,9 +1350,18 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
@{ Shape = 'a white-space string'; Value = ' '; ValueText = $null }
@{ Shape = 'an undefined number'; Value = 99; ValueText = '99' }
@{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null }
@{ Shape = 'a comma list of two names'; Value = 'Allow,Deny'; ValueText = 'Allow,Deny' }
@{ Shape = 'an array of two names'; Value = @('Allow', 'Deny'); ValueText = 'Allow, Deny' }
)
# The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (5 + 3 * 5) = 40 rows.
# AccessType and AppliesTo name one value each, not a combination: PowerShell would add the numbers of a list, and
# an entry for a scope that the caller did not name would follow.
$accessAppliesToListShapes = @(
@{ Shape = 'a comma list of two names'; Value = 'ThisFolderAndSubfolders,ThisFolderAndFiles'; ValueText = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ Shape = 'an array of two names'; Value = @('ThisFolderAndSubfolders', 'ThisFolderAndFiles'); ValueText = 'ThisFolderAndSubfolders, ThisFolderAndFiles' }
)
# The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (7 + 3 * 5 + 2) = 48 rows.
$accessInvalidParameterCases = foreach ($cmdlet in 'Add-NTFSAccess', 'Remove-NTFSAccess') {
foreach ($shapeCase in $accessTypeBadShapes) {
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Shape = $shapeCase.Shape; Value = $shapeCase.Value; ValueText = $shapeCase.ValueText }
@ -1363,6 +1371,9 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
@{ Cmdlet = $cmdlet; Parameter = $parameterName; Shape = $shapeCase.Shape; Value = $shapeCase.Value; ValueText = $shapeCase.ValueText }
}
}
foreach ($shapeCase in $accessAppliesToListShapes) {
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Shape = $shapeCase.Shape; Value = $shapeCase.Value; ValueText = $shapeCase.ValueText }
}
}
$accessSdInvalidParameterCases = foreach ($cmdlet in 'Add-NTFSAccess', 'Remove-NTFSAccess') {
@ -1382,6 +1393,10 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
@{ Cmdlet = $cmdlet; Parameter = 'InheritanceFlags'; Value = 'Typoo' }
@{ Cmdlet = $cmdlet; Parameter = 'PropagationFlags'; Value = 'Typoo' }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'Typoo' }
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = 'Allow,Deny' }
@{ Cmdlet = $cmdlet; Parameter = 'AccessType'; Value = @('Allow', 'Deny') }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = @('ThisFolderAndSubfolders', 'ThisFolderAndFiles') }
}
# A named $null (a literal $null, a variable, an alias, splatting, or $PSDefaultParameterValues) must
@ -1399,6 +1414,7 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
@{ Description = 'a different case'; Value = 'deny' }
@{ Description = 'the enum value'; Value = [System.Security.AccessControl.AccessControlType]::Deny }
@{ Description = 'an integer'; Value = 1 }
@{ Description = 'an array of one name'; Value = @('Deny') }
)
$accessValidFlagsShapeCases = @(
@ -1432,6 +1448,14 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
@{ Parameter = 'AppliesTo'; Word = 'ThisFolderO'; ExpectedName = 'ThisFolderOnly' }
)
# In Turkish, a case-insensitive comparison of the culture of the thread treats I and i as different letters, so
# a word typed in capitals completes only when the completers compare culture invariant.
$accessTurkishCompletionCases = @(
@{ Parameter = 'AppliesTo'; Word = 'THISFOLDERO'; ExpectedName = 'ThisFolderOnly' }
@{ Parameter = 'InheritanceFlags'; Word = 'CONTAINERINH'; ExpectedName = 'ContainerInherit' }
@{ Parameter = 'PropagationFlags'; Word = 'NOPROPAGATEINHERI'; ExpectedName = 'NoPropagateInherit' }
)
# Because the five parameters are object, PowerShell counts a $null, an empty string, or DBNull value of a
# property as present when it chooses the parameter set, the same as a real value, so AppliesTo (the Simple
# group) together with a $null/''/DBNull InheritanceFlags or PropagationFlags (the Complex group) offers
@ -1879,6 +1903,23 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
$r4 = [System.Management.Automation.CommandCompletion]::CompleteInput($wildcard, $wildcard.Length, $null)
@($r4.CompletionMatches.CompletionText) | Should -Contain $ExpectedName
}
It 'Should complete Add-NTFSAccess -<Parameter> for the word <Word> while the culture of the thread is Turkish' -ForEach $accessTurkishCompletionCases {
$inputScript = 'Add-NTFSAccess -Path C:\x -Account a -AccessRights Read -{0} {1}' -f $Parameter, $Word
$original = [System.Threading.Thread]::CurrentThread.CurrentCulture
try {
[System.Threading.Thread]::CurrentThread.CurrentCulture = [System.Globalization.CultureInfo]::GetCultureInfo('tr-TR')
'I'.ToLower() | Should -Be ([string][char]0x131) -Because 'the culture of the thread has to be Turkish for the test to mean anything'
$completion = [System.Management.Automation.CommandCompletion]::CompleteInput($inputScript, $inputScript.Length, $null)
$names = @($completion.CompletionMatches | ForEach-Object -Process { $_.CompletionText })
}
finally {
[System.Threading.Thread]::CurrentThread.CurrentCulture = $original
}
$names | Should -Contain $ExpectedName
}
}
}

43
Tests/ParameterSets.Audit.Tests.ps1

@ -3,11 +3,7 @@
the audit cmdlets: pipeline binding by value and by property name, continuation past a failing path or
descriptor, and state or output assertions beyond -PassThru, with the module built in NTFSSecurity\bin\Release
on files in a sandbox folder. Reading and changing audit entries needs the Security privilege; tests that need
it skip without it and run in CI, whose runners are elevated, like Audit.Tests.ps1. The -SecurityDescriptor sets
of Add-NTFSAudit and Remove-NTFSAudit have no per-item handler around the change itself (an open maintainer
item), so no test here pins what happens when that change throws for one of several descriptors; the only
failure the -SecurityDescriptor scenarios below exercise is the already-handled one of a descriptor that was
read without its audit section.
it skip without it and run in CI, whose runners are elevated, like Audit.Tests.ps1.
#>
[Diagnostics.CodeAnalysis.SuppressMessageAttribute(
'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.'
@ -1023,7 +1019,14 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
@{ Shape = 'a hashtable'; Value = @{ Unrelated = 1 }; ValueText = $null }
)
# The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (5 + 3 * 5) = 40 rows.
# The cross product of the two cmdlets that share these parameters and every bad shape: 2 * (5 + 3 * 5 + 2) = 42 rows.
# AppliesTo names one value, not a combination: PowerShell would add the numbers of a list, and an entry for a
# scope that the caller did not name would follow. AuditFlags and the two flags parameters take a list.
$auditAppliesToListShapes = @(
@{ Shape = 'a comma list of two names'; Value = 'ThisFolderAndSubfolders,ThisFolderAndFiles'; ValueText = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ Shape = 'an array of two names'; Value = @('ThisFolderAndSubfolders', 'ThisFolderAndFiles'); ValueText = 'ThisFolderAndSubfolders, ThisFolderAndFiles' }
)
$auditInvalidParameterCases = foreach ($cmdlet in 'Add-NTFSAudit', 'Remove-NTFSAudit') {
foreach ($shapeCase in $auditFlagsBadShapes) {
@{ Cmdlet = $cmdlet; Parameter = 'AuditFlags'; Shape = $shapeCase.Shape; Value = $shapeCase.Value; ValueText = $shapeCase.ValueText }
@ -1033,6 +1036,9 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
@{ Cmdlet = $cmdlet; Parameter = $parameterName; Shape = $shapeCase.Shape; Value = $shapeCase.Value; ValueText = $shapeCase.ValueText }
}
}
foreach ($shapeCase in $auditAppliesToListShapes) {
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Shape = $shapeCase.Shape; Value = $shapeCase.Value; ValueText = $shapeCase.ValueText }
}
}
$auditSdInvalidParameterCases = foreach ($cmdlet in 'Add-NTFSAudit', 'Remove-NTFSAudit') {
@ -1052,6 +1058,8 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
@{ Cmdlet = $cmdlet; Parameter = 'InheritanceFlags'; Value = 'Typoo' }
@{ Cmdlet = $cmdlet; Parameter = 'PropagationFlags'; Value = 'Typoo' }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'Typoo' }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'ThisFolderAndSubfolders,ThisFolderAndFiles' }
@{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = @('ThisFolderAndSubfolders', 'ThisFolderAndFiles') }
}
# A named $null (a literal $null, a variable, splatting, or $PSDefaultParameterValues) must stay a
@ -1100,6 +1108,12 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
@{ Parameter = 'AuditFlags'; Word = 'F'; ExpectedName = 'Failure' }
)
# In Turkish, a case-insensitive comparison of the culture of the thread treats I and i as different letters, so
# a word typed in capitals completes only when the completers compare culture invariant.
$auditTurkishCompletionCases = @(
@{ Parameter = 'AuditFlags'; Word = 'FAIL'; ExpectedName = 'Failure' }
)
# Because the five parameters are object, PowerShell counts a $null, an empty string, or DBNull value of a
# property as present when it chooses the parameter set, the same as a real value, so AppliesTo (the Simple
# group) together with a $null/''/DBNull InheritanceFlags or PropagationFlags (the Complex group) offers
@ -1549,5 +1563,22 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
$r4 = [System.Management.Automation.CommandCompletion]::CompleteInput($wildcard, $wildcard.Length, $null)
@($r4.CompletionMatches.CompletionText) | Should -Contain $ExpectedName
}
It 'Should complete Add-NTFSAudit -<Parameter> for the word <Word> while the culture of the thread is Turkish' -ForEach $auditTurkishCompletionCases {
$inputScript = 'Add-NTFSAudit -Path C:\x -Account a -AccessRights Read -{0} {1}' -f $Parameter, $Word
$original = [System.Threading.Thread]::CurrentThread.CurrentCulture
try {
[System.Threading.Thread]::CurrentThread.CurrentCulture = [System.Globalization.CultureInfo]::GetCultureInfo('tr-TR')
'I'.ToLower() | Should -Be ([string][char]0x131) -Because 'the culture of the thread has to be Turkish for the test to mean anything'
$completion = [System.Management.Automation.CommandCompletion]::CompleteInput($inputScript, $inputScript.Length, $null)
$names = @($completion.CompletionMatches | ForEach-Object -Process { $_.CompletionText })
}
finally {
[System.Threading.Thread]::CurrentThread.CurrentCulture = $original
}
$names | Should -Contain $ExpectedName
}
}
}

Loading…
Cancel
Save