Fix firewall analysis showing "-" instead of actual blocked domains for iptables-dropped traffic (#20016)

Author: Copilot

actions/setup/js/firewall_blocked_domains.cjs

@@ -155,7 +155,10 @@ function getBlockedDomains(logsDir) {
       // Check if request was blocked
       const isBlocked = isRequestBlocked(entry.decision, entry.status);
       if (isBlocked) {
-        const sanitizedDomain = extractAndSanitizeDomain(entry.domain);
+        // When domain is "-" (iptables-dropped traffic not visible to Squid),
+        // fall back to dest IP:port so blocked requests show their actual destination instead of "-"
+        const domainField = entry.domain !== "-" ? entry.domain : entry.destIpPort;
+        const sanitizedDomain = extractAndSanitizeDomain(domainField);
         if (sanitizedDomain && sanitizedDomain !== "-") {
           blockedDomainsSet.add(sanitizedDomain);
         }

actions/setup/js/firewall_blocked_domains.test.cjs

@@ -230,7 +230,34 @@ describe("firewall_blocked_domains.cjs", () => {
 
       const result = getBlockedDomains(logsDir);
 
-      expect(result).toEqual(["blocked.example.com"]);
+      // The iptables-dropped entry uses destIpPort (140.82.112.22) as fallback
+      expect(result).toContain("blocked.example.com");
+      expect(result).toContain("140.82.112.22");
+    });
+
+    it("should use destIpPort as fallback when domain is placeholder", () => {
+      const logsDir = path.join(testDir, "logs-iptables");
+      fs.mkdirSync(logsDir, { recursive: true });
+
+      // Simulate iptables-dropped traffic: domain="-", destIpPort has actual destination
+      const logContent = [
+        '1761332530.474 172.30.0.20:35288 - 8.8.8.8:53 - - 0 NONE_NONE:HIER_NONE - "-"', // iptables-dropped DNS query
+        '1761332530.475 172.30.0.20:35289 - 1.2.3.4:443 - - 0 NONE_NONE:HIER_NONE - "-"', // iptables-dropped HTTPS
+        '1761332530.476 172.30.0.20:35290 - - - - 0 NONE_NONE:HIER_NONE - "-"', // truly unknown (both domain and destIpPort are "-")
+        '1761332530.477 172.30.0.20:35291 allowed.example.com:443 5.5.5.5:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT allowed.example.com:443 "-"', // allowed request
+      ].join("\n");
+
+      fs.writeFileSync(path.join(logsDir, "access.log"), logContent);
+
+      const result = getBlockedDomains(logsDir);
+
+      // iptables-dropped entries should use destIpPort as domain identifier
+      expect(result).toContain("8.8.8.8");
+      expect(result).toContain("1.2.3.4");
+      // truly unknown (both domain and destIpPort are "-") should be excluded
+      expect(result).not.toContain("-");
+      // allowed domains should not appear
+      expect(result).not.toContain("allowed.example.com");
     });
 
     it("should handle invalid log lines gracefully", () => {

actions/setup/js/git_patch_integration.test.cjs

@@ -11,13 +11,22 @@
  * These tests require git to be installed and create temporary git repos.
  */
 
-import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
 import fs from "fs";
 import path from "path";
 import { spawnSync } from "child_process";
 import os from "os";
 import { generateGitPatch } from "./generate_git_patch.cjs";
 
+// generateGitPatch uses execGitSync from git_helpers.cjs which calls core.debug / core.error
+// as GitHub Actions globals. Provide a no-op mock so these tests work outside of Actions.
+global.core = {
+  debug: vi.fn(),
+  error: vi.fn(),
+  info: vi.fn(),
+  warning: vi.fn(),
+};
+
 /**
  * Execute git command safely with args array
  */

actions/setup/js/parse_firewall_logs.cjs

@@ -57,19 +57,23 @@ async function main() {
         // Determine if request was allowed or blocked
         const isAllowed = isRequestAllowed(entry.decision, entry.status);
 
+        // When domain is "-" (iptables-dropped traffic not visible to Squid),
+        // fall back to dest IP:port so blocked requests show their actual destination instead of "-"
+        const domainKey = entry.domain !== "-" ? entry.domain : entry.destIpPort !== "-" ? entry.destIpPort : "-";
+
         if (isAllowed) {
           allowedRequests++;
-          allowedDomains.add(entry.domain);
+          allowedDomains.add(domainKey);
         } else {
           blockedRequests++;
-          blockedDomains.add(entry.domain);
+          blockedDomains.add(domainKey);
         }
 
         // Track request count per domain
-        if (!requestsByDomain.has(entry.domain)) {
-          requestsByDomain.set(entry.domain, { allowed: 0, blocked: 0 });
+        if (!requestsByDomain.has(domainKey)) {
+          requestsByDomain.set(domainKey, { allowed: 0, blocked: 0 });
         }
-        const domainStats = requestsByDomain.get(entry.domain);
+        const domainStats = requestsByDomain.get(domainKey);
         if (isAllowed) {
           domainStats.allowed++;
         } else {

cmd/gh-aw/main.go

@@ -216,7 +216,7 @@ Examples:
 
 var compileCmd = &cobra.Command{
 	Use:   "compile [workflow]...",
-	Short: "Compile workflow markdown files (.md) into GitHub Actions workflows (.lock.yml)",
+	Short: "Compile workflow Markdown files (.md) into GitHub Actions workflows (.lock.yml)",
 	Long: `Compile one or more agentic workflows to YAML workflows.
 
 If no workflows are specified, all Markdown files in .github/workflows will be compiled.

pkg/cli/firewall_log.go

@@ -252,8 +252,12 @@ func parseFirewallLog(logPath string, verbose bool) (*FirewallAnalysis, error) {
 		// Determine if request was allowed or blocked
 		isAllowed := isRequestAllowed(entry.Decision, entry.Status)
 
-		// Extract domain (remove port)
+		// Extract domain - when domain is "-" (iptables-dropped traffic not visible to Squid),
+		// fall back to dest IP:port so blocked requests show their actual destination instead of "-"
 		domain := entry.Domain
+		if domain == "-" && entry.DestIPPort != "-" {
+			domain = entry.DestIPPort
+		}
 
 		if isAllowed {
 			analysis.AllowedRequests++

pkg/cli/firewall_log_test.go

@@ -453,6 +453,76 @@ func TestParseFirewallLogPartialMissingFields(t *testing.T) {
 	}
 }
 
+func TestParseFirewallLogIptablesDropped(t *testing.T) {
+	// Create a temporary directory for the test
+	tempDir := testutil.TempDir(t, "test-*")
+
+	// Simulate iptables-dropped traffic: domain="-" but destIPPort has the actual destination.
+	// This occurs when iptables drops packets before they reach the Squid proxy, so Squid
+	// only sees the IP layer info and logs domain as "-".
+	testLogContent := `1761332530.474 172.30.0.20:35288 api.github.com:443 140.82.112.22:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"
+1761332531.123 172.30.0.20:35289 - 8.8.8.8:53 - - 0 NONE_NONE:HIER_NONE - "-"
+1761332532.456 172.30.0.20:35290 - 1.2.3.4:443 - - 0 NONE_NONE:HIER_NONE - "-"
+1761332533.789 172.30.0.20:35291 - 1.2.3.4:443 - - 0 NONE_NONE:HIER_NONE - "-"
+1761332534.012 172.30.0.20:35292 - - - - 0 NONE_NONE:HIER_NONE - "-"
+`
+
+	// Write test log file
+	logPath := filepath.Join(tempDir, "firewall.log")
+	err := os.WriteFile(logPath, []byte(testLogContent), 0644)
+	if err != nil {
+		t.Fatalf("Failed to create test firewall.log: %v", err)
+	}
+
+	// Test parsing
+	analysis, err := parseFirewallLog(logPath, false)
+	if err != nil {
+		t.Fatalf("Failed to parse firewall log: %v", err)
+	}
+
+	if analysis.TotalRequests != 5 {
+		t.Errorf("TotalRequests: got %d, want 5", analysis.TotalRequests)
+	}
+	if analysis.AllowedRequests != 1 {
+		t.Errorf("AllowedRequests: got %d, want 1", analysis.AllowedRequests)
+	}
+	if analysis.BlockedRequests != 4 {
+		t.Errorf("BlockedRequests: got %d, want 4", analysis.BlockedRequests)
+	}
+
+	// Iptables-dropped entries with destIPPort should use destIPPort as the key
+	if stats, ok := analysis.RequestsByDomain["8.8.8.8:53"]; !ok {
+		t.Error("8.8.8.8:53 should be in RequestsByDomain (iptables-dropped fallback)")
+	} else if stats.Blocked != 1 {
+		t.Errorf("8.8.8.8:53 Blocked: got %d, want 1", stats.Blocked)
+	}
+
+	if stats, ok := analysis.RequestsByDomain["1.2.3.4:443"]; !ok {
+		t.Error("1.2.3.4:443 should be in RequestsByDomain (iptables-dropped fallback)")
+	} else if stats.Blocked != 2 {
+		t.Errorf("1.2.3.4:443 Blocked: got %d, want 2", stats.Blocked)
+	}
+
+	// "-" should only appear for entries where both domain and destIPPort are "-"
+	if stats, ok := analysis.RequestsByDomain["-"]; !ok {
+		t.Error("\"-\" should be in RequestsByDomain for truly-unknown entries")
+	} else if stats.Blocked != 1 {
+		t.Errorf("\"-\" Blocked: got %d, want 1", stats.Blocked)
+	}
+
+	// BlockedDomains should include the real IPs, not just "-"
+	blockedSet := make(map[string]bool)
+	for _, d := range analysis.BlockedDomains {
+		blockedSet[d] = true
+	}
+	if !blockedSet["8.8.8.8:53"] {
+		t.Error("BlockedDomains should contain 8.8.8.8:53 (iptables-dropped fallback)")
+	}
+	if !blockedSet["1.2.3.4:443"] {
+		t.Error("BlockedDomains should contain 1.2.3.4:443 (iptables-dropped fallback)")
+	}
+}
+
 func TestAnalyzeMultipleFirewallLogs(t *testing.T) {
 	// Create a temporary directory for the test
 	tempDir := testutil.TempDir(t, "test-*")

pkg/cli/logs_parsing_firewall.go

@@ -163,19 +163,24 @@ const originalMain = function() {
         // Determine if request was allowed or blocked
         const isAllowed = isRequestAllowed(entry.decision, entry.status);
 
+        // When domain is "-" (iptables-dropped traffic not visible to Squid),
+        // fall back to dest IP:port so blocked requests show their actual destination instead of "-"
+        const domainKey =
+          entry.domain !== "-" ? entry.domain : entry.destIpPort !== "-" ? entry.destIpPort : "-";
+
         if (isAllowed) {
           allowedRequests++;
-          allowedDomains.add(entry.domain);
+          allowedDomains.add(domainKey);
         } else {
           blockedRequests++;
-          blockedDomains.add(entry.domain);
+          blockedDomains.add(domainKey);
         }
 
         // Track request count per domain
-        if (!requestsByDomain.has(entry.domain)) {
-          requestsByDomain.set(entry.domain, { allowed: 0, blocked: 0 });
+        if (!requestsByDomain.has(domainKey)) {
+          requestsByDomain.set(domainKey, { allowed: 0, blocked: 0 });
         }
-        const domainStats = requestsByDomain.get(entry.domain);
+        const domainStats = requestsByDomain.get(domainKey);
         if (isAllowed) {
           domainStats.allowed++;
         } else {