Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion actions/setup/js/firewall_blocked_domains.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
29 changes: 28 additions & 1 deletion actions/setup/js/firewall_blocked_domains.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
11 changes: 10 additions & 1 deletion actions/setup/js/git_patch_integration.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
14 changes: 9 additions & 5 deletions actions/setup/js/parse_firewall_logs.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/gh-aw/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion pkg/cli/firewall_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++
Expand Down
70 changes: 70 additions & 0 deletions pkg/cli/firewall_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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-*")
Expand Down
15 changes: 10 additions & 5 deletions pkg/cli/logs_parsing_firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading