Add scoped unspecified firewall rule handling and allow empty rules

.github/workflows/rust.yml

@@ -304,11 +304,16 @@ jobs:
 
   coverage-report:
     if: github.event_name == 'pull_request'
-    # Use Linux coverage only: merging all platforms inflates total line count
-    # because each platform has platform-specific source files (Windows adds ~4500
-    # lines from registry/service/DISM resources). Single-platform coverage matches
-    # local `build.ps1 -codecoverage` results and avoids misleadingly low percentages.
-    needs: [linux-build, linux-pester]
+    # Use all platforms for changed-code coverage so platform-specific files are
+    # included. Keep full-codebase coverage Linux-only to avoid inflating its
+    # denominator with platform-specific sources.
+    needs:
+      - linux-build
+      - linux-pester
+      - macos-build
+      - macos-pester
+      - windows-build
+      - windows-pester
     runs-on: ubuntu-latest
     permissions:
       pull-requests: write
@@ -321,7 +326,7 @@ jobs:
       - name: Download coverage artifacts
         uses: actions/download-artifact@v4
         with:
-          pattern: 'linux*coverage'
+          pattern: '*coverage'
           path: coverage-data
 
       - name: Consolidate coverage data
@@ -339,30 +344,49 @@ jobs:
               $baseSha = $mergeBase
           }
 
-          # Find all available lcov.info files from coverage artifacts
+          # Changed-code coverage uses every platform so platform-specific Rust
+          # files are analyzed. Full-codebase coverage remains Linux-only to
+          # avoid inflating its denominator with platform-specific sources.
           $lcovFiles = Get-ChildItem -Path 'coverage-data' -Filter 'lcov.info' -Recurse
           $pesterLcovFiles = Get-ChildItem -Path 'coverage-data' -Filter 'pester-lcov.info' -Recurse
           $allLcovFiles = @($lcovFiles) + @($pesterLcovFiles) | Where-Object { $_ }
+          $linuxLcovFiles = @($allLcovFiles | Where-Object {
+              ($_.FullName -match '[/\\]linux-[^/\\]+-coverage[/\\]') -or
+              ($_.FullName -match '[/\\]linux-coverage[/\\]')
+          })
 
           if ($allLcovFiles.Count -eq 0) {
               Write-Warning 'No coverage data found from any platform.'
               "coverage_failed=true" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT
               return
           }
+          if ($linuxLcovFiles.Count -eq 0) {
+              Write-Warning 'No Linux coverage data found for the full-codebase report.'
+              "coverage_failed=true" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT
+              return
+          }
           "coverage_failed=false" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT
 
-          Write-Verbose -Verbose "Found $($allLcovFiles.Count) LCOV file(s) to merge"
+          Write-Verbose -Verbose "Found $($allLcovFiles.Count) cross-platform LCOV file(s)"
+          Write-Verbose -Verbose "Found $($linuxLcovFiles.Count) Linux LCOV file(s)"
 
-          # Merge all LCOV files into a single consolidated report
+          # Merge all platforms for changed-code coverage.
           $mergedLcovPath = Join-Path $PWD 'merged-lcov.info'
           if ($allLcovFiles.Count -eq 1) {
               Copy-Item -Path $allLcovFiles[0].FullName -Destination $mergedLcovPath
           } else {
               Merge-LcovFile -Path ($allLcovFiles | ForEach-Object { $_.FullName }) -OutputPath $mergedLcovPath -Verbose
           }
 
-          # Full codebase coverage report (always computed)
-          $fullReport = Get-FullCodeCoverageReport -LcovPath $mergedLcovPath -Verbose
+          # Merge Linux coverage separately for the full-codebase report.
+          $linuxMergedLcovPath = Join-Path $PWD 'linux-merged-lcov.info'
+          if ($linuxLcovFiles.Count -eq 1) {
+              Copy-Item -Path $linuxLcovFiles[0].FullName -Destination $linuxMergedLcovPath
+          } else {
+              Merge-LcovFile -Path ($linuxLcovFiles | ForEach-Object { $_.FullName }) -OutputPath $linuxMergedLcovPath -Verbose
+          }
+
+          $fullReport = Get-FullCodeCoverageReport -LcovPath $linuxMergedLcovPath -Verbose
 
           "full_percentage=$($fullReport.Percentage)" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT
           "full_covered=$($fullReport.CoveredLines)" | Out-File -Append -Encoding utf8 -FilePath $env:GITHUB_OUTPUT

resources/windows_firewall/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "windows_firewall"
-version = "0.2.0"
+version = "0.3.0"
 edition = "2024"
 
 [package.metadata.i18n]

resources/windows_firewall/locales/en-us.toml

@@ -9,11 +9,9 @@ invalidJson = "Invalid JSON input: %{error}"
 windowsOnly = "This resource is only supported on Windows"
 
 [get]
-rulesArrayEmpty = "The rules array cannot be empty for get operations"
 selectorRequired = "Each firewall rule in a get request must include a name"
 
 [set]
-rulesArrayEmpty = "The rules array cannot be empty for set operations"
 selectorRequired = "Each firewall rule in a set request must include a name"
 
 [firewall]
@@ -28,8 +26,10 @@ ruleUpdateFailed = "Failed to update firewall rule '%{name}': %{error}"
 ruleReadFailed = "Failed to read firewall rule '%{name}': %{error}"
 portsNotAllowed = "Ports cannot be specified for firewall rule '%{name}' because protocol %{protocol} does not support ports"
 invalidProfiles = "Invalid profiles value '%{value}'. Valid values are Domain, Private, Public, or All"
+emptyUnspecifiedProfiles = "The unspecified rules profiles filter cannot be empty"
 invalidInterfaceType = "Invalid interface type '%{value}'. Valid values are RemoteAccess, Wireless, Lan, or All"
 invalidProtocol = "Invalid protocol number '%{value}'. Must be between 0 and 256"
+variantClearFailed = "Warning: VariantClear failed with HRESULT: %{hresult}"
 
 [firewall_helper]
 whatIfCreateRule = "Would create firewall rule '%{name}'"

resources/windows_firewall/src/firewall.rs

@@ -10,7 +10,10 @@ use windows::Win32::System::Com::{CLSCTX_INPROC_SERVER, CoCreateInstance, CoInit
 use windows::Win32::System::Ole::IEnumVARIANT;
 use windows::Win32::System::Variant::{VARIANT, VariantClear};
 
-use crate::types::{FirewallError, FirewallRule, FirewallRuleList, Metadata, RuleAction, RuleDirection, UnspecifiedRulesAction};
+use crate::types::{
+    FirewallError, FirewallRule, FirewallRuleList, Metadata, RuleAction, RuleDirection,
+    UnspecifiedRuleAction, UnspecifiedRules,
+};
 
 /// RAII wrapper for VARIANT that automatically calls VariantClear on drop
 struct SafeVariant(VARIANT);
@@ -32,7 +35,13 @@ impl SafeVariant {
 impl Drop for SafeVariant {
     fn drop(&mut self) {
         if let Err(e) = unsafe { VariantClear(&mut self.0) } {
-            crate::write_error(&format!("Warning: VariantClear failed with HRESULT: {:#010x}", e.code().0 as u32));
+            crate::write_error(
+                t!(
+                    "firewall.variantClearFailed",
+                    hresult = format!("{:#010x}", e.code().0 as u32)
+                )
+                .as_ref(),
+            );
         }
     }
 }
@@ -212,6 +221,30 @@ fn profiles_to_mask(values: &[String]) -> Result<i32, FirewallError> {
     Ok(mask)
 }
 
+fn rule_matches_unspecified_scope(
+    rule: &FirewallRule,
+    unspecified_rules: &UnspecifiedRules,
+) -> Result<bool, FirewallError> {
+    if let Some(direction) = unspecified_rules.direction.as_ref()
+        && rule.direction.as_ref() != Some(direction)
+    {
+        return Ok(false);
+    }
+
+    if let Some(profiles) = unspecified_rules.profiles.as_ref() {
+        if profiles.is_empty() {
+            return Err(t!("firewall.emptyUnspecifiedProfiles").to_string().into());
+        }
+        let requested_mask = profiles_to_mask(profiles)?;
+        let rule_mask = profiles_to_mask(rule.profiles.as_deref().unwrap_or_default())?;
+        if requested_mask & rule_mask == 0 {
+            return Ok(false);
+        }
+    }
+
+    Ok(true)
+}
+
 fn split_csv(value: Option<String>) -> Option<Vec<String>> {
     value.map(|raw| {
         raw.split(',')
@@ -381,10 +414,6 @@ fn apply_rule_properties(rule: &INetFwRule, desired: &FirewallRule, existing_pro
 }
 
 pub fn get_rules(input: &FirewallRuleList) -> Result<FirewallRuleList, FirewallError> {
-    if input.rules.is_empty() {
-        return Err(t!("get.rulesArrayEmpty").to_string().into());
-    }
-
     let store = FirewallStore::open()?;
     let mut results = Vec::new();
 
@@ -399,7 +428,7 @@ pub fn get_rules(input: &FirewallRuleList) -> Result<FirewallRuleList, FirewallE
         }
     }
 
-    Ok(FirewallRuleList { rules: results, unspecified_rules_action: None })
+    Ok(FirewallRuleList { rules: results, unspecified_rules: None })
 }
 
 fn project_rule(current: &FirewallRule, desired: &FirewallRule) -> FirewallRule {
@@ -426,10 +455,6 @@ fn project_rule(current: &FirewallRule, desired: &FirewallRule) -> FirewallRule
 }
 
 pub fn set_rules(input: &FirewallRuleList, what_if: bool) -> Result<FirewallRuleList, FirewallError> {
-    if input.rules.is_empty() {
-        return Err(t!("set.rulesArrayEmpty").to_string().into());
-    }
-
     let store = FirewallStore::open()?;
     let mut results = Vec::new();
 
@@ -496,10 +521,14 @@ pub fn set_rules(input: &FirewallRuleList, what_if: bool) -> Result<FirewallRule
         }
     }
 
-    // Handle unspecified_rules_action: Disable or Remove rules not explicitly listed
-    match &input.unspecified_rules_action {
-        Some(UnspecifiedRulesAction::Disable) | Some(UnspecifiedRulesAction::Remove) => {
-            let is_remove = matches!(&input.unspecified_rules_action, Some(UnspecifiedRulesAction::Remove));
+    // Disable or remove rules that aren't explicitly listed and match the requested scope.
+    match &input.unspecified_rules {
+        Some(unspecified_rules) if matches!(
+            unspecified_rules.action,
+            UnspecifiedRuleAction::Disable | UnspecifiedRuleAction::Remove
+        ) =>
+        {
+            let is_remove = unspecified_rules.action == UnspecifiedRuleAction::Remove;
             let specified_names: std::collections::HashSet<String> = input.rules.iter()
                 .filter_map(|r| r.selector_name().map(|n| n.to_ascii_lowercase()))
                 .collect();
@@ -516,11 +545,15 @@ pub fn set_rules(input: &FirewallRuleList, what_if: bool) -> Result<FirewallRule
                 if rule_name.starts_with("ms-resource://") {
                     continue;
                 }
-                
+
                 if specified_names.contains(&rule_name.to_ascii_lowercase()) {
                     continue;
                 }
 
+                if !rule_matches_unspecified_scope(&model, unspecified_rules)? {
+                    continue;
+                }
+
                 if is_remove {
                     if what_if {
                         let mut projected = model.missing_from_input();
@@ -551,10 +584,10 @@ pub fn set_rules(input: &FirewallRuleList, what_if: bool) -> Result<FirewallRule
                 }
             }
         }
-        _ => {} // None or Ignore — no additional action
+        _ => {} // None or Ignore: no additional action.
     }
 
-    Ok(FirewallRuleList { rules: results, unspecified_rules_action: input.unspecified_rules_action.clone() })
+    Ok(FirewallRuleList { rules: results, unspecified_rules: input.unspecified_rules.clone() })
 }
 
 pub fn export_rules() -> Result<FirewallRuleList, FirewallError> {
@@ -566,5 +599,74 @@ pub fn export_rules() -> Result<FirewallRuleList, FirewallError> {
         results.push(rule_to_model(&rule)?);
     }
 
-    Ok(FirewallRuleList { rules: results, unspecified_rules_action: None })
+    Ok(FirewallRuleList { rules: results, unspecified_rules: None })
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn rule(direction: RuleDirection, profiles: &[&str]) -> FirewallRule {
+        FirewallRule {
+            direction: Some(direction),
+            profiles: Some(
+                profiles
+                    .iter()
+                    .map(|profile| (*profile).to_string())
+                    .collect(),
+            ),
+            ..FirewallRule::default()
+        }
+    }
+
+    fn scope(direction: Option<RuleDirection>, profiles: Option<&[&str]>) -> UnspecifiedRules {
+        UnspecifiedRules {
+            action: UnspecifiedRuleAction::Disable,
+            direction,
+            profiles: profiles.map(|values| {
+                values
+                    .iter()
+                    .map(|profile| (*profile).to_string())
+                    .collect()
+            }),
+        }
+    }
+
+    #[test]
+    fn unspecified_rule_scope_combines_direction_and_profiles() {
+        let filter = scope(Some(RuleDirection::Inbound), Some(&["Domain"]));
+
+        assert!(
+            rule_matches_unspecified_scope(&rule(RuleDirection::Inbound, &["Domain"]), &filter)
+                .unwrap()
+        );
+        assert!(
+            !rule_matches_unspecified_scope(&rule(RuleDirection::Outbound, &["Domain"]), &filter)
+                .unwrap()
+        );
+        assert!(
+            !rule_matches_unspecified_scope(&rule(RuleDirection::Inbound, &["Private"]), &filter)
+                .unwrap()
+        );
+    }
+
+    #[test]
+    fn unspecified_rule_profile_scope_intersects_all_profiles() {
+        let filter = scope(None, Some(&["Domain"]));
+
+        assert!(
+            rule_matches_unspecified_scope(&rule(RuleDirection::Inbound, &["All"]), &filter)
+                .unwrap()
+        );
+    }
+
+    #[test]
+    fn unspecified_rule_profile_scope_rejects_empty_filter() {
+        let filter = scope(None, Some(&[]));
+
+        assert!(
+            rule_matches_unspecified_scope(&rule(RuleDirection::Inbound, &["Domain"]), &filter)
+                .is_err()
+        );
+    }
 }

resources/windows_firewall/src/types.rs

@@ -23,17 +23,29 @@ pub enum RuleAction {
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Clone)]
 #[serde(rename_all = "camelCase")]
-pub enum UnspecifiedRulesAction {
+pub enum UnspecifiedRuleAction {
     Ignore,
     Disable,
     Remove,
 }
 
+#[derive(Debug, Serialize, Deserialize, Clone)]
+#[serde(rename_all = "camelCase")]
+pub struct UnspecifiedRules {
+    pub action: UnspecifiedRuleAction,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub direction: Option<RuleDirection>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub profiles: Option<Vec<String>>,
+}
+
 #[derive(Debug, Default, Serialize, Deserialize, Clone)]
 #[serde(rename_all = "camelCase")]
 pub struct FirewallRuleList {
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub unspecified_rules_action: Option<UnspecifiedRulesAction>,
+    pub unspecified_rules: Option<UnspecifiedRules>,
     pub rules: Vec<FirewallRule>,
 }
 
@@ -135,6 +147,22 @@ impl From<String> for FirewallError {
 #[cfg(windows)]
 impl From<windows::core::Error> for FirewallError {
     fn from(error: windows::core::Error) -> Self {
-        Self { message: error.to_string() }
+        Self {
+            message: error.to_string(),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::FirewallRuleList;
+
+    #[test]
+    fn unspecified_rules_requires_action() {
+        let result = serde_json::from_str::<FirewallRuleList>(
+            r#"{"unspecifiedRules":{"direction":"Inbound"},"rules":[]}"#,
+        );
+
+        assert!(result.is_err());
     }
 }