Add --parse support for firewall logs in logs and audit commands#2349
Add --parse support for firewall logs in logs and audit commands#2349
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Agentic Changeset Generator triggered by this pull request. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds firewall log parsing capability to the logs and audit commands via the --parse flag. When enabled, both agent and firewall logs are parsed, generating log.md and firewall.md respectively.
Key Changes:
- Added
parseFirewallLogsfunction that wraps an embedded JavaScript parser - Integrated firewall parsing into existing
--parseflag workflows - Added comprehensive test coverage for firewall log parsing scenarios
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/logs.go | Implements parseFirewallLogs function and integrates it into the logs command's parse workflow |
| pkg/cli/audit.go | Integrates firewall log parsing into the audit command's parse workflow |
| pkg/cli/logs_firewall_parse_test.go | Adds test coverage for firewall log parsing in various scenarios (with logs, without logs, empty directories) |
| docs/src/content/docs/reference/frontmatter-full.md | Removes outdated firewall feature flag documentation |
| .github/workflows/research.lock.yml | Updates firewall log parser with improved validation and filtered domain display |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (domain !== "-" && !/^[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+$/.test(domain)) { | ||
|
|
||
| return null; |
There was a problem hiding this comment.
[nitpick] Multiple validation checks return null on failure without logging why parsing failed. Consider adding debug logging to indicate which field validation failed, as this would help diagnose parsing issues in production firewall logs.
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const domain = fields[2]; | ||
|
|
||
| if (domain !== "-" && !/^[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+$/.test(domain)) { | ||
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const destIpPort = fields[3]; | ||
|
|
||
| if (destIpPort !== "-" && !/^[\d.]+:\d+$/.test(destIpPort)) { | ||
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const status = fields[6]; | ||
|
|
||
| if (status !== "-" && !/^\d+$/.test(status)) { | ||
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const decision = fields[7]; | ||
|
|
||
| if (decision !== "-" && !decision.includes(":")) { | ||
|
|
||
| return null; | ||
|
|
There was a problem hiding this comment.
[nitpick] Multiple validation checks return null on failure without logging why parsing failed. Consider adding debug logging to indicate which field validation failed, as this would help diagnose parsing issues in production firewall logs.
| return null; | |
| } | |
| const domain = fields[2]; | |
| if (domain !== "-" && !/^[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+$/.test(domain)) { | |
| return null; | |
| } | |
| const destIpPort = fields[3]; | |
| if (destIpPort !== "-" && !/^[\d.]+:\d+$/.test(destIpPort)) { | |
| return null; | |
| } | |
| const status = fields[6]; | |
| if (status !== "-" && !/^\d+$/.test(status)) { | |
| return null; | |
| } | |
| const decision = fields[7]; | |
| if (decision !== "-" && !decision.includes(":")) { | |
| return null; | |
| console.debug(`Firewall log parse failed: invalid clientIpPort "${clientIpPort}"`); | |
| return null; | |
| } | |
| const domain = fields[2]; | |
| if (domain !== "-" && !/^[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+$/.test(domain)) { | |
| console.debug(`Firewall log parse failed: invalid domain "${domain}"`); | |
| return null; | |
| } | |
| const destIpPort = fields[3]; | |
| if (destIpPort !== "-" && !/^[\d.]+:\d+$/.test(destIpPort)) { | |
| console.debug(`Firewall log parse failed: invalid destIpPort "${destIpPort}"`); | |
| return null; | |
| } | |
| const status = fields[6]; | |
| if (status !== "-" && !/^\d+$/.test(status)) { | |
| console.debug(`Firewall log parse failed: invalid status "${status}"`); | |
| return null; | |
| } | |
| const decision = fields[7]; | |
| if (decision !== "-" && !decision.includes(":")) { | |
| console.debug(`Firewall log parse failed: invalid decision "${decision}"`); | |
| return null; |
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const domain = fields[2]; | ||
|
|
||
| if (domain !== "-" && !/^[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+$/.test(domain)) { | ||
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const destIpPort = fields[3]; | ||
|
|
||
| if (destIpPort !== "-" && !/^[\d.]+:\d+$/.test(destIpPort)) { | ||
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const status = fields[6]; | ||
|
|
||
| if (status !== "-" && !/^\d+$/.test(status)) { | ||
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const decision = fields[7]; | ||
|
|
||
| if (decision !== "-" && !decision.includes(":")) { | ||
|
|
||
| return null; | ||
|
|
There was a problem hiding this comment.
[nitpick] Multiple validation checks return null on failure without logging why parsing failed. Consider adding debug logging to indicate which field validation failed, as this would help diagnose parsing issues in production firewall logs.
| return null; | |
| } | |
| const domain = fields[2]; | |
| if (domain !== "-" && !/^[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+$/.test(domain)) { | |
| return null; | |
| } | |
| const destIpPort = fields[3]; | |
| if (destIpPort !== "-" && !/^[\d.]+:\d+$/.test(destIpPort)) { | |
| return null; | |
| } | |
| const status = fields[6]; | |
| if (status !== "-" && !/^\d+$/.test(status)) { | |
| return null; | |
| } | |
| const decision = fields[7]; | |
| if (decision !== "-" && !decision.includes(":")) { | |
| return null; | |
| console.debug(`Firewall log parse failed: invalid clientIpPort "${clientIpPort}"`); | |
| return null; | |
| } | |
| const domain = fields[2]; | |
| if (domain !== "-" && !/^[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+$/.test(domain)) { | |
| console.debug(`Firewall log parse failed: invalid domain "${domain}"`); | |
| return null; | |
| } | |
| const destIpPort = fields[3]; | |
| if (destIpPort !== "-" && !/^[\d.]+:\d+$/.test(destIpPort)) { | |
| console.debug(`Firewall log parse failed: invalid destIpPort "${destIpPort}"`); | |
| return null; | |
| } | |
| const status = fields[6]; | |
| if (status !== "-" && !/^\d+$/.test(status)) { | |
| console.debug(`Firewall log parse failed: invalid status "${status}"`); | |
| return null; | |
| } | |
| const decision = fields[7]; | |
| if (decision !== "-" && !decision.includes(":")) { | |
| console.debug(`Firewall log parse failed: invalid decision "${decision}"`); | |
| return null; |
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const status = fields[6]; | ||
|
|
||
| if (status !== "-" && !/^\d+$/.test(status)) { | ||
|
|
||
| return null; | ||
|
|
||
| } | ||
|
|
||
| const decision = fields[7]; | ||
|
|
||
| if (decision !== "-" && !decision.includes(":")) { | ||
|
|
||
| return null; | ||
|
|
There was a problem hiding this comment.
[nitpick] Multiple validation checks return null on failure without logging why parsing failed. Consider adding debug logging to indicate which field validation failed, as this would help diagnose parsing issues in production firewall logs.
| return null; | |
| } | |
| const status = fields[6]; | |
| if (status !== "-" && !/^\d+$/.test(status)) { | |
| return null; | |
| } | |
| const decision = fields[7]; | |
| if (decision !== "-" && !decision.includes(":")) { | |
| return null; | |
| console.debug(`Parsing failed: destIpPort field invalid ('${destIpPort}'). Expected format: IP:port or '-'.`); | |
| return null; | |
| } | |
| const status = fields[6]; | |
| if (status !== "-" && !/^\d+$/.test(status)) { | |
| console.debug(`Parsing failed: status field invalid ('${status}'). Expected numeric or '-'.`); | |
| return null; | |
| } | |
| const decision = fields[7]; | |
| if (decision !== "-" && !decision.includes(":")) { | |
| console.debug(`Parsing failed: decision field invalid ('${decision}'). Expected to contain ':' or be '-'.`); | |
| return null; |
| jsScript := workflow.GetLogParserScript("parse_firewall_logs") | ||
| if jsScript == "" { | ||
| if verbose { | ||
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Failed to get firewall log parser script")) |
There was a problem hiding this comment.
The error message 'Failed to get firewall log parser script' doesn't explain why it failed or what the user should do. Consider adding context such as 'Firewall log parser script not found in embedded resources' or similar to help with troubleshooting.
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Failed to get firewall log parser script")) | |
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Firewall log parser script not found in embedded resources (parse_firewall_logs)")) |
Update CLI documentation to reflect recent feature additions: - Document firewall log parsing in --parse flag (PR #2349, #2350) - Logs and audit commands now generate firewall.md files - JSON output includes firewall analysis - Update --dependabot documentation (PR #2359) - Added pip and Go ecosystem support - Clarified command detection patterns - Add repository feature validation section (PR #2347) - Compile validates discussions/issues enabled - Prevents runtime failures for incompatible workflows 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The
--parseflag now parses firewall logs in addition to agent logs, generatingfirewall.mdalongsidelog.md.Changes
Added
parseFirewallLogsfunction inpkg/cli/logs.goparse_firewall_logs.cjsscript with mocked GitHub Actions environmentsquid-logs/orworkflow-logs/squid-logs/directoriesIntegrated firewall parsing into
logsandauditcommands--parseis usedfirewall.mdUpdated help text to document firewall log parsing in both commands
Usage
The generated
firewall.mdcontains blocked domain statistics and request counts.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.