Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the CatNet core engine by consolidating several legacy packages into two main packages: pkg/scanner and pkg/exporter. It introduces a cleaner API for concurrent network scanning, target range parsing, and secure data exporting (JSON, CSV, XML) with CSV injection prevention. Additionally, it updates documentation, downgrades the Go version in go.mod to 1.25.0, and adds comprehensive unit tests. The review feedback highlights critical improvements: refactoring StartScan to accept a context.Context to avoid non-reentrant package-level globals, optimizing Windows MAC resolution by caching DLL loading at the package level, and checking writer.Error() after flushing the CSV writer to ensure data integrity.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func StartScan(ips []string, cfg ScanConfig, onResult func(DeviceInfo), onProgress func(float64)) error { | ||
| if !isScanning.CompareAndSwap(false, true) { | ||
| return fmt.Errorf("scan already in progress") | ||
| } | ||
| defer isScanning.Store(false) | ||
|
|
||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| scanMu.Lock() | ||
| cancelScan = cancel | ||
| scanMu.Unlock() | ||
| defer func() { | ||
| scanMu.Lock() | ||
| cancelScan = nil | ||
| scanMu.Unlock() | ||
| cancel() | ||
| }() |
There was a problem hiding this comment.
Using package-level global variables (isScanning, scanMu, cancelScan) to manage scanning state and cancellation makes the package non-reentrant and unsafe for concurrent use (e.g., running multiple independent scans in parallel or running parallel tests). Additionally, hardcoding context.Background() and not accepting a context.Context parameter is non-idiomatic in Go for long-running network operations.
Consider refactoring StartScan to accept a context.Context as its first parameter. This allows the caller to handle cancellation and timeouts natively using standard Go contexts, eliminating the need for global cancellation state.
| func StartScan(ips []string, cfg ScanConfig, onResult func(DeviceInfo), onProgress func(float64)) error { | |
| if !isScanning.CompareAndSwap(false, true) { | |
| return fmt.Errorf("scan already in progress") | |
| } | |
| defer isScanning.Store(false) | |
| ctx, cancel := context.WithCancel(context.Background()) | |
| scanMu.Lock() | |
| cancelScan = cancel | |
| scanMu.Unlock() | |
| defer func() { | |
| scanMu.Lock() | |
| cancelScan = nil | |
| scanMu.Unlock() | |
| cancel() | |
| }() | |
| func StartScan(ctx context.Context, ips []string, cfg ScanConfig, onResult func(DeviceInfo), onProgress func(float64)) error { | |
| if !isScanning.CompareAndSwap(false, true) { | |
| return fmt.Errorf("scan already in progress") | |
| } | |
| defer isScanning.Store(false) | |
| ctx, cancel := context.WithCancel(ctx) | |
| scanMu.Lock() | |
| cancelScan = cancel | |
| scanMu.Unlock() | |
| defer func() { | |
| scanMu.Lock() | |
| cancelScan = nil | |
| scanMu.Unlock() | |
| cancel() | |
| }() |
| // osGetMAC obtém o MAC usando SendARP no Windows | ||
| func osGetMAC(ip string) string { | ||
| iphlpapi := syscall.NewLazyDLL("iphlpapi.dll") | ||
| sendARP := iphlpapi.NewProc("SendARP") | ||
|
|
||
| destIP := net.ParseIP(ip).To4() |
There was a problem hiding this comment.
Calling syscall.NewLazyDLL and iphlpapi.NewProc inside osGetMAC on every invocation is highly inefficient. Since osGetMAC is called for every active host during a scan, loading the DLL and looking up the procedure repeatedly introduces unnecessary overhead. Moving these to package-level variables ensures they are initialized only once.
| // osGetMAC obtém o MAC usando SendARP no Windows | |
| func osGetMAC(ip string) string { | |
| iphlpapi := syscall.NewLazyDLL("iphlpapi.dll") | |
| sendARP := iphlpapi.NewProc("SendARP") | |
| destIP := net.ParseIP(ip).To4() | |
| var ( | |
| iphlpapi = syscall.NewLazyDLL("iphlpapi.dll") | |
| sendARP = iphlpapi.NewProc("SendARP") | |
| ) | |
| // osGetMAC obtém o MAC usando SendARP no Windows | |
| func osGetMAC(ip string) string { | |
| destIP := net.ParseIP(ip).To4() |
| writer.Flush() | ||
| return buf.Bytes(), nil |
There was a problem hiding this comment.
In Go, csv.Writer buffers writes. Calling writer.Flush() writes any buffered data to the underlying writer, but it does not return an error. To ensure no errors occurred during flushing or writing, you should check writer.Error() after calling Flush().
writer.Flush()
if err := writer.Error(); err != nil {
return nil, err
}
return buf.Bytes(), nil
Summary
This PR introduces the foundational v0.1.0 release of
catnet-core, the shared Go engine for the CatNet scanning ecosystem. It provides reusable packages for network scanning, host discovery, result export (JSON/CSV/XML), and scanning lifecycle management — without any GUI/TUI/presentation-layer code.Type of change
Key additions
pkg/scannerDeviceInfo,ScanConfig,DefaultConfig,SanitizevalidateIPv4,Ping,ReverseDNS,GetMAC,ScanPortsParseRangewith CIDR and dash range supportStartScan,StopScanwith goroutines and context-based cancellationpkg/exporterExportJSON,ExportCSV,ExportXMLwith CSV injection sanitizationCI/CD
go vet+go test -racein GitHub Actionsgovulncheckfor vulnerability scanningValidation
TestValidateIPv4,TestExportCSV,TestExportXML,TestExportJSON,TestScanConcurrency)Statistics
pkg/scan,pkg/export,pkg/errors,pkg/events,pkg/profile,pkg/results,pkg/targets) refactored into consolidatedpkg/scannerandpkg/exporter