Skip to content

Commit a795c26

Browse files
author
Steve Lee (POWERSHELL HE/HIM) (from Dev Box)
committed
address copilot feedback
1 parent f55e7ad commit a795c26

5 files changed

Lines changed: 18 additions & 11 deletions

File tree

dsc/tests/dsc_resource_list.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ BeforeDiscovery {
1111

1212
Describe 'Tests for listing resources' {
1313
It 'dsc resource list' {
14-
$resources = dsc resource list | ConvertFrom-Json -Depth 10
14+
$resources = dsc resource list | ConvertFrom-Json -Depth 15
1515
$LASTEXITCODE | Should -Be 0
1616
$resources | Should -Not -BeNullOrEmpty
1717
$resources.Count | Should -BeGreaterThan 0

resources/windows_firewall/src/firewall.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -263,17 +263,24 @@ fn apply_rule_properties(rule: &INetFwRule, desired: &FirewallRule, existing_pro
263263

264264
if let Some(protocol) = desired.protocol {
265265
validate_protocol(protocol)?;
266+
}
267+
268+
// Determine the effective protocol: the desired value if provided, otherwise
269+
// the existing rule's protocol (if updating an existing rule).
270+
let effective_protocol = desired.protocol.or(existing_protocol);
266271

267-
// Reject port specifications for protocols that don't support them (e.g. ICMP).
268-
// On existing rules the ports are cleared automatically, but on new rules
269-
// (existing_protocol == None) the conflicting SetLocalPorts/SetRemotePorts call
270-
// would fail with a confusing COM error.
271-
if !protocol_supports_ports(protocol) && existing_protocol.is_none()
272+
// Reject port specifications for protocols that don't support them (e.g. ICMP).
273+
// This must be checked regardless of whether the protocol itself was changed,
274+
// because the caller may only be setting local_ports or remote_ports.
275+
if let Some(protocol) = effective_protocol {
276+
if !protocol_supports_ports(protocol)
272277
&& (desired.local_ports.is_some() || desired.remote_ports.is_some())
273278
{
274279
return Err(t!("firewall.portsNotAllowed", name = name, protocol = protocol).to_string().into());
275280
}
281+
}
276282

283+
if let Some(protocol) = desired.protocol {
277284
if let Some(current_protocol) = existing_protocol
278285
&& current_protocol != protocol && !protocol_supports_ports(protocol) {
279286
if desired.local_ports.is_none() {

resources/windows_firewall/tests/windows_firewall_export.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Copyright (c) Microsoft Corporation.
22
# Licensed under the MIT License.
33

4-
Describe 'Microsoft.Windows/FirewallRuleList - export operation' -Skip:(!$IsWindows) {
4+
Describe 'Microsoft.Windows/FirewallRuleList - export operation' -Skip:(!$isElevated) {
55
BeforeDiscovery {
66
$isElevated = if ($IsWindows) {
77
([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole(

resources/windows_firewall/tests/windows_firewall_get.tests.ps1

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Copyright (c) Microsoft Corporation.
22
# Licensed under the MIT License.
33

4-
Describe 'Microsoft.Windows/FirewallRuleList - get operation' -Skip:(!$IsWindows) {
4+
Describe 'Microsoft.Windows/FirewallRuleList - get operation' -Skip:(!$isElevated) {
55
BeforeDiscovery {
66
$isElevated = if ($IsWindows) {
77
([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole(
@@ -35,7 +35,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - get operation' -Skip:(!$IsWindows
3535

3636
$result = ($out | ConvertFrom-Json).actualState.rules[0]
3737
$result.name | Should -BeExactly $knownRuleName
38-
$result.PSObject.Properties.Name | Should -Not -Contain '_exist'
38+
$result.PSObject.Properties.Name | Should -Not -Contain '_exist' -Because ($result | ConvertTo-Json -Depth 10)
3939
$result.direction | Should -BeIn @('Inbound', 'Outbound')
4040
$result.action | Should -BeIn @('Allow', 'Block')
4141
}
@@ -46,7 +46,7 @@ Describe 'Microsoft.Windows/FirewallRuleList - get operation' -Skip:(!$IsWindows
4646
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log)
4747

4848
$result = ($out | ConvertFrom-Json).actualState.rules[0]
49-
$result.PSObject.Properties.Name | Should -Not -Contain '_exist'
49+
$result.PSObject.Properties.Name | Should -Not -Contain '_exist' -Because ($result | ConvertTo-Json -Depth 10)
5050
$result.name | Should -BeExactly $knownRuleName
5151
}
5252

resources/windows_firewall/tests/windows_firewall_set.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Copyright (c) Microsoft Corporation.
22
# Licensed under the MIT License.
33

4-
Describe 'Microsoft.Windows/FirewallRuleList - set operation' -Skip:(!$IsWindows) {
4+
Describe 'Microsoft.Windows/FirewallRuleList - set operation' -Skip:(!$isElevated) {
55
BeforeDiscovery {
66
$isElevated = if ($IsWindows) {
77
([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole(

0 commit comments

Comments
 (0)