Skip to content

Fix firewall log parsing by relaxing field validation to match JavaScript parser #9227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
Jan 7, 2026
Merged

Fix firewall log parsing by relaxing field validation to match JavaScript parser #9227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9cf8560
Initial plan
Copilot Jan 7, 2026
addcb50
Initial analysis: Firewall log parsing is broken due to overly strict…
Copilot Jan 7, 2026
6825a7e
Fix firewall log parsing by relaxing validation to match JavaScript p…
Copilot Jan 7, 2026
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
1 change: 0 additions & 1 deletion .github/workflows/dictation-prompt.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 5 additions & 40 deletions pkg/cli/firewall_log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,8 @@ var (
// - Comment lines (starting with #) are skipped
// - Empty lines are skipped
// - Lines with fewer than 10 fields are rejected
// - Field validation uses regex patterns matching the JavaScript parser:
// * timestamp: must be numeric with optional decimal point
// * client_ip:port: must be IP:port format or "-"
// * domain: must be domain:port format or "-"
// * dest_ip:port: must be IP:port format or "-"
// * status: must be numeric or "-"
// * decision: must contain ":" or be "-"
// - Only timestamp field is validated (must be numeric with optional decimal point)
// - Other fields are accepted as-is without validation (matches JavaScript parser behavior)
// - User agent quotes are automatically stripped
//
// # Request Classification
Expand Down Expand Up @@ -164,49 +159,19 @@ func parseFirewallLogLine(line string) *FirewallLogEntry {
return nil
}

// Validate timestamp format (should be numeric with optional decimal point)
// Only validate timestamp (essential for log format detection)
// This matches the JavaScript parser behavior which only validates timestamp
timestamp := fields[0]
if matched, _ := regexp.MatchString(`^\d+(\.\d+)?$`, timestamp); !matched {
return nil
}

// Validate client IP:port format (should be IP:port or "-")
// Extract fields without validation (matches JavaScript parser)
clientIPPort := fields[1]
if clientIPPort != "-" {
if matched, _ := regexp.MatchString(`^[\d.]+:\d+$`, clientIPPort); !matched {
return nil
}
}

// Validate domain format (should be domain:port or "-")
domain := fields[2]
if domain != "-" {
if matched, _ := regexp.MatchString(`^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*:\d+$`, domain); !matched {
return nil
}
}

// Validate dest IP:port format (should be IP:port or "-")
destIPPort := fields[3]
if destIPPort != "-" {
if matched, _ := regexp.MatchString(`^[\d.]+:\d+$`, destIPPort); !matched {
return nil
}
}

// Validate status code (should be numeric or "-")
status := fields[6]
if status != "-" {
if matched, _ := regexp.MatchString(`^\d+$`, status); !matched {
return nil
}
}

// Validate decision format (should contain ":" or be "-")
decision := fields[7]
if decision != "-" && !strings.Contains(decision, ":") {
return nil
}

// Remove quotes from user agent
userAgent := fields[9]
Expand Down
113 changes: 92 additions & 21 deletions pkg/cli/firewall_log_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,39 +64,105 @@ func TestParseFirewallLogLine(t *testing.T) {
expected: nil,
},
{
name: "invalid client IP:port format",
line: `1761332530.474 Accepting api.github.com:443 140.82.112.22:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: nil,
name: "non-standard client IP:port format is accepted",
line: `1761332530.474 Accepting api.github.com:443 140.82.112.22:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: &FirewallLogEntry{
Timestamp: "1761332530.474",
ClientIPPort: "Accepting",
Domain: "api.github.com:443",
DestIPPort: "140.82.112.22:443",
Proto: "1.1",
Method: "CONNECT",
Status: "200",
Decision: "TCP_TUNNEL:HIER_DIRECT",
URL: "api.github.com:443",
UserAgent: "-",
},
},
{
name: "invalid domain format (no port)",
line: `1761332530.474 172.30.0.20:35288 DNS 140.82.112.22:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: nil,
name: "non-standard domain format is accepted",
line: `1761332530.474 172.30.0.20:35288 DNS 140.82.112.22:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: &FirewallLogEntry{
Timestamp: "1761332530.474",
ClientIPPort: "172.30.0.20:35288",
Domain: "DNS",
DestIPPort: "140.82.112.22:443",
Proto: "1.1",
Method: "CONNECT",
Status: "200",
Decision: "TCP_TUNNEL:HIER_DIRECT",
URL: "api.github.com:443",
UserAgent: "-",
},
},
{
name: "invalid dest IP:port format",
line: `1761332530.474 172.30.0.20:35288 api.github.com:443 Local 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: nil,
name: "non-standard dest IP:port format is accepted",
line: `1761332530.474 172.30.0.20:35288 api.github.com:443 Local 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: &FirewallLogEntry{
Timestamp: "1761332530.474",
ClientIPPort: "172.30.0.20:35288",
Domain: "api.github.com:443",
DestIPPort: "Local",
Proto: "1.1",
Method: "CONNECT",
Status: "200",
Decision: "TCP_TUNNEL:HIER_DIRECT",
URL: "api.github.com:443",
UserAgent: "-",
},
},
{
name: "invalid status code (non-numeric)",
line: `1761332530.474 172.30.0.20:35288 api.github.com:443 140.82.112.22:443 1.1 CONNECT Swap TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: nil,
name: "non-numeric status code is accepted",
line: `1761332530.474 172.30.0.20:35288 api.github.com:443 140.82.112.22:443 1.1 CONNECT Swap TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: &FirewallLogEntry{
Timestamp: "1761332530.474",
ClientIPPort: "172.30.0.20:35288",
Domain: "api.github.com:443",
DestIPPort: "140.82.112.22:443",
Proto: "1.1",
Method: "CONNECT",
Status: "Swap",
Decision: "TCP_TUNNEL:HIER_DIRECT",
URL: "api.github.com:443",
UserAgent: "-",
},
},
{
name: "invalid decision format (no colon)",
line: `1761332530.474 172.30.0.20:35288 api.github.com:443 140.82.112.22:443 1.1 CONNECT 200 Waiting api.github.com:443 "-"`,
expected: nil,
name: "decision format without colon is accepted",
line: `1761332530.474 172.30.0.20:35288 api.github.com:443 140.82.112.22:443 1.1 CONNECT 200 Waiting api.github.com:443 "-"`,
expected: &FirewallLogEntry{
Timestamp: "1761332530.474",
ClientIPPort: "172.30.0.20:35288",
Domain: "api.github.com:443",
DestIPPort: "140.82.112.22:443",
Proto: "1.1",
Method: "CONNECT",
Status: "200",
Decision: "Waiting",
URL: "api.github.com:443",
UserAgent: "-",
},
},
{
name: "fewer than 10 fields",
line: `WARNING: Something went wrong`,
expected: nil,
},
{
name: "line with pipe character in domain position",
line: `1761332530.474 172.30.0.20:35288 pinger|test 140.82.112.22:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: nil,
name: "line with pipe character in domain position is accepted",
line: `1761332530.474 172.30.0.20:35288 pinger|test 140.82.112.22:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"`,
expected: &FirewallLogEntry{
Timestamp: "1761332530.474",
ClientIPPort: "172.30.0.20:35288",
Domain: "pinger|test",
DestIPPort: "140.82.112.22:443",
Proto: "1.1",
Method: "CONNECT",
Status: "200",
Decision: "TCP_TUNNEL:HIER_DIRECT",
URL: "api.github.com:443",
UserAgent: "-",
},
},
}

Expand Down Expand Up @@ -334,14 +400,19 @@ Invalid line with not enough fields
t.Fatalf("Failed to parse firewall log: %v", err)
}

// Should only have parsed 2 valid lines
if analysis.TotalRequests != 2 {
t.Errorf("TotalRequests: got %d, want 2 (should skip malformed lines)", analysis.TotalRequests)
// Should have parsed 3 valid lines (relaxed validation accepts INVALID_IP like JavaScript parser)
// Lines with valid timestamps and 10 fields are accepted, even if field formats are non-standard
if analysis.TotalRequests != 3 {
t.Errorf("TotalRequests: got %d, want 3 (non-standard formats accepted)", analysis.TotalRequests)
}

if analysis.AllowedRequests != 2 {
t.Errorf("AllowedRequests: got %d, want 2", analysis.AllowedRequests)
}

if analysis.BlockedRequests != 1 {
t.Errorf("BlockedRequests: got %d, want 1", analysis.BlockedRequests)
}
}

func TestParseFirewallLogPartialMissingFields(t *testing.T) {
Expand Down