diff --git a/actions/setup/js/firewall_blocked_domains.cjs b/actions/setup/js/firewall_blocked_domains.cjs index 6432ecb1da3..c23e472310f 100644 --- a/actions/setup/js/firewall_blocked_domains.cjs +++ b/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); } diff --git a/actions/setup/js/firewall_blocked_domains.test.cjs b/actions/setup/js/firewall_blocked_domains.test.cjs index f809374c460..3f74d556995 100644 --- a/actions/setup/js/firewall_blocked_domains.test.cjs +++ b/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", () => { diff --git a/actions/setup/js/git_patch_integration.test.cjs b/actions/setup/js/git_patch_integration.test.cjs index 8a09f507cb2..584d601edf5 100644 --- a/actions/setup/js/git_patch_integration.test.cjs +++ b/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 */ diff --git a/actions/setup/js/parse_firewall_logs.cjs b/actions/setup/js/parse_firewall_logs.cjs index fc69fdadce4..956c99457aa 100644 --- a/actions/setup/js/parse_firewall_logs.cjs +++ b/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 { diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index c043abeac15..1ce23131789 100644 --- a/cmd/gh-aw/main.go +++ b/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. diff --git a/pkg/cli/firewall_log.go b/pkg/cli/firewall_log.go index 4ce83a905d1..348311a0893 100644 --- a/pkg/cli/firewall_log.go +++ b/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++ diff --git a/pkg/cli/firewall_log_test.go b/pkg/cli/firewall_log_test.go index 576d1497e27..e7bbb6836e2 100644 --- a/pkg/cli/firewall_log_test.go +++ b/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-*") diff --git a/pkg/cli/logs_parsing_firewall.go b/pkg/cli/logs_parsing_firewall.go index 87686bcd889..1c4a87b78ff 100644 --- a/pkg/cli/logs_parsing_firewall.go +++ b/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 {