code_public/ja4sentinel
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: architecture violations and pre-existing test bugs

Browse Source 
- Remove JA4SENTINEL_LOG_LEVEL env override (architecture violation: log_level must be YAML-only)
- Add TestLoadFromEnv_LogLevelIgnored test to verify env var is ignored
- Fix yaml struct tags in api.Config/AppConfig/OutputConfig (yaml.v3 ignores json tags)
- Fix isValidIP/isValidCIDR to use net.ParseIP/net.ParseCIDR for proper validation
- Fix SLL packet parsing: use protoType from SLL header to select IPv4/IPv6 decoder
- Fix TestLoadFromFile_ExcludeSourceIPs: t.Errorf → t.Fatalf to avoid nil dereference
- Fix TestFromClientHello_NilPayload: use strings.HasPrefix for error message check
- Fix TestValidate_ExcludeSourceIPs: add required FlowTimeoutSec/PacketBufferSize defaults

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
toto
2026-03-05 09:22:29 +01:00
parent bd45344d19
commit e9e523d8a2
5 changed files with 94 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
internal/config/loader.go
View File

@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"net"
"os"
"strconv"
"strings"
@ -104,10 +105,8 @@ func (l *LoaderImpl) loadFromEnv(config api.AppConfig) api.AppConfig {
}
}
// JA4SENTINEL_LOG_LEVEL
if val := os.Getenv("JA4SENTINEL_LOG_LEVEL"); val != "" {
config.Core.LogLevel = val
}
// Note: JA4SENTINEL_LOG_LEVEL is intentionally NOT loaded from env.
// log_level must be configured exclusively via the YAML config file.
return config
}
@ -284,46 +283,13 @@ func ToJSON(config api.AppConfig) string {
return string(data)
}
// isValidIP checks if a string is a valid IP address
// isValidIP checks if a string is a valid IP address using net.ParseIP
func isValidIP(ip string) bool {
if ip == "" {
return false
}
// Simple validation: check if it contains only valid IP characters
for _, ch := range ip {
if !((ch >= '0' && ch <= '9') || ch == '.') {
// Could be IPv6
if ch == ':' {
return true // Accept IPv6 without detailed validation
}
return false
}
}
return true
return net.ParseIP(ip) != nil
}
// isValidCIDR checks if a string is a valid CIDR notation
// isValidCIDR checks if a string is a valid CIDR notation using net.ParseCIDR
func isValidCIDR(cidr string) bool {
if cidr == "" {
return false
}
parts := strings.Split(cidr, "/")
if len(parts) != 2 {
return false
}
// Check IP part
if !isValidIP(parts[0]) {
return false
}
// Check prefix length
prefix, err := strconv.Atoi(parts[1])
if err != nil {
return false
}
if strings.Contains(parts[0], ":") {
// IPv6
return prefix >= 0 && prefix <= 128
}
// IPv4
return prefix >= 0 && prefix <= 32
_, _, err := net.ParseCIDR(cidr)
return err == nil
}

40
internal/config/loader_test.go
View File

@ -583,14 +583,6 @@ func TestLoadFromEnv_InvalidValues(t *testing.T) {
wantErr: true, // Validation error
errContains: "flow_timeout_sec must be between",
},
{
name: "invalid_log_level",
env: map[string]string{
"JA4SENTINEL_LOG_LEVEL": "invalid-level",
},
wantErr: true, // Validation error
errContains: "log_level must be one of",
},
}
for _, tt := range tests {
@ -637,7 +629,6 @@ func TestLoadFromEnv_AllValidValues(t *testing.T) {
t.Setenv("JA4SENTINEL_BPF_FILTER", "tcp port 8443")
t.Setenv("JA4SENTINEL_FLOW_TIMEOUT", "60")
t.Setenv("JA4SENTINEL_PACKET_BUFFER_SIZE", "2000")
t.Setenv("JA4SENTINEL_LOG_LEVEL", "debug")
loader := NewLoader("")
cfg, err := loader.Load()
@ -661,9 +652,6 @@ func TestLoadFromEnv_AllValidValues(t *testing.T) {
if cfg.Core.PacketBufferSize != 2000 {
t.Errorf("PacketBufferSize = %d, want 2000", cfg.Core.PacketBufferSize)
}
if cfg.Core.LogLevel != "debug" {
t.Errorf("LogLevel = %q, want 'debug'", cfg.Core.LogLevel)
}
}
// TestValidate_WhitespaceOnlyInterface tests that whitespace-only interface is rejected
@ -826,7 +814,7 @@ outputs:
}
if len(cfg.Core.ExcludeSourceIPs) != 2 {
t.Errorf("ExcludeSourceIPs length = %d, want 2", len(cfg.Core.ExcludeSourceIPs))
t.Fatalf("ExcludeSourceIPs length = %d, want 2", len(cfg.Core.ExcludeSourceIPs))
}
if cfg.Core.ExcludeSourceIPs[0] != "10.0.0.0/8" {
t.Errorf("ExcludeSourceIPs[0] = %q, want '10.0.0.0/8'", cfg.Core.ExcludeSourceIPs[0])
@ -955,6 +943,8 @@ func TestValidate_ExcludeSourceIPs(t *testing.T) {
Core: api.Config{
Interface: "eth0",
ListenPorts: []uint16{443},
FlowTimeoutSec: api.DefaultFlowTimeout,
PacketBufferSize: api.DefaultPacketBuffer,
ExcludeSourceIPs: tt.ips,
},
Outputs: []api.OutputConfig{
@ -981,7 +971,8 @@ func TestValidate_ExcludeSourceIPs(t *testing.T) {
func TestLoadFromEnv_ExcludeSourceIPs_NotSupported(t *testing.T) {
// This test documents that exclude_source_ips is NOT loaded from env
// It's only loaded from config file
t.Setenv("JA4SENTINEL_LOG_LEVEL", "debug")
t.Setenv("JA4SENTINEL_INTERFACE", "lo")
t.Setenv("JA4SENTINEL_PORTS", "443")
loader := NewLoader("")
cfg, err := loader.Load()
@ -994,9 +985,24 @@ func TestLoadFromEnv_ExcludeSourceIPs_NotSupported(t *testing.T) {
if len(cfg.Core.ExcludeSourceIPs) != 0 {
t.Errorf("ExcludeSourceIPs should be empty from env, got %v", cfg.Core.ExcludeSourceIPs)
}
}
// But log_level should be loaded from env
if cfg.Core.LogLevel != "debug" {
t.Errorf("LogLevel = %q, want 'debug'", cfg.Core.LogLevel)
// TestLoadFromEnv_LogLevelIgnored verifies that JA4SENTINEL_LOG_LEVEL env var is NOT honored.
// log_level must be configured exclusively via the YAML config file (architecture requirement).
func TestLoadFromEnv_LogLevelIgnored(t *testing.T) {
t.Setenv("JA4SENTINEL_INTERFACE", "lo")
t.Setenv("JA4SENTINEL_PORTS", "443")
t.Setenv("JA4SENTINEL_LOG_LEVEL", "debug")
loader := NewLoader("")
cfg, err := loader.Load()
if err != nil {
t.Fatalf("Load() error = %v", err)
}
// log_level must NOT be overridden by env var; the default ("info") applies
if cfg.Core.LogLevel == "debug" {
t.Error("LogLevel should NOT be set from JA4SENTINEL_LOG_LEVEL env var")
}
}