fix: renforcer corrélation A/B et sorties stdout/fichier
Co-authored-by: aider (openrouter/openai/gpt-5.3-codex) <aider@aider.chat>
This commit is contained in:
@ -4,7 +4,9 @@ import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
@ -115,35 +117,37 @@ func (s *ClickHouseSink) Name() string {
|
||||
|
||||
// Write adds a log to the buffer.
|
||||
func (s *ClickHouseSink) Write(ctx context.Context, log domain.CorrelatedLog) error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
deadline := time.Now().Add(time.Duration(s.config.TimeoutMs) * time.Millisecond)
|
||||
|
||||
// Check buffer overflow
|
||||
if len(s.buffer) >= s.config.MaxBufferSize {
|
||||
if s.config.DropOnOverflow {
|
||||
// Drop the log
|
||||
for {
|
||||
s.mu.Lock()
|
||||
if len(s.buffer) < s.config.MaxBufferSize {
|
||||
s.buffer = append(s.buffer, log)
|
||||
if len(s.buffer) >= s.config.BatchSize {
|
||||
select {
|
||||
case s.flushChan <- struct{}{}:
|
||||
default:
|
||||
}
|
||||
}
|
||||
s.mu.Unlock()
|
||||
return nil
|
||||
}
|
||||
// Block until space is available (with timeout)
|
||||
drop := s.config.DropOnOverflow
|
||||
s.mu.Unlock()
|
||||
|
||||
if drop {
|
||||
return nil
|
||||
}
|
||||
if time.Now().After(deadline) {
|
||||
return fmt.Errorf("buffer full, timeout exceeded")
|
||||
}
|
||||
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
case <-time.After(time.Duration(s.config.TimeoutMs) * time.Millisecond):
|
||||
return fmt.Errorf("buffer full, timeout exceeded")
|
||||
case <-time.After(10 * time.Millisecond):
|
||||
}
|
||||
}
|
||||
|
||||
s.buffer = append(s.buffer, log)
|
||||
|
||||
// Trigger flush if batch is full
|
||||
if len(s.buffer) >= s.config.BatchSize {
|
||||
select {
|
||||
case s.flushChan <- struct{}{}:
|
||||
default:
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// Flush flushes the buffer to ClickHouse.
|
||||
@ -311,7 +315,33 @@ func isRetryableError(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
if errors.Is(err, context.DeadlineExceeded) {
|
||||
return true
|
||||
}
|
||||
|
||||
if errors.Is(err, context.Canceled) {
|
||||
return false
|
||||
}
|
||||
|
||||
var netErr net.Error
|
||||
if errors.As(err, &netErr) {
|
||||
if netErr.Timeout() {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
errStr := strings.ToLower(err.Error())
|
||||
|
||||
// Explicit non-retryable SQL/schema errors
|
||||
if strings.Contains(errStr, "syntax error") ||
|
||||
strings.Contains(errStr, "unknown table") ||
|
||||
strings.Contains(errStr, "unknown column") ||
|
||||
(strings.Contains(errStr, "table") && strings.Contains(errStr, "not found")) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Fallback network/transient errors
|
||||
retryableErrors := []string{
|
||||
"connection refused",
|
||||
"connection reset",
|
||||
@ -319,11 +349,13 @@ func isRetryableError(err error) bool {
|
||||
"temporary failure",
|
||||
"network is unreachable",
|
||||
"broken pipe",
|
||||
"no route to host",
|
||||
}
|
||||
for _, re := range retryableErrors {
|
||||
if strings.Contains(errStr, re) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
@ -1,7 +1,6 @@
|
||||
package file
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
@ -30,7 +29,6 @@ type FileSink struct {
|
||||
config Config
|
||||
mu sync.Mutex
|
||||
file *os.File
|
||||
writer *bufio.Writer
|
||||
}
|
||||
|
||||
// NewFileSink creates a new file sink.
|
||||
@ -66,11 +64,12 @@ func (s *FileSink) Write(ctx context.Context, log domain.CorrelatedLog) error {
|
||||
return fmt.Errorf("failed to marshal log: %w", err)
|
||||
}
|
||||
|
||||
if _, err := s.writer.Write(data); err != nil {
|
||||
return fmt.Errorf("failed to write log: %w", err)
|
||||
line := append(data, '\n')
|
||||
if _, err := s.file.Write(line); err != nil {
|
||||
return fmt.Errorf("failed to write log line: %w", err)
|
||||
}
|
||||
if _, err := s.writer.WriteString("\n"); err != nil {
|
||||
return fmt.Errorf("failed to write newline: %w", err)
|
||||
if err := s.file.Sync(); err != nil {
|
||||
return fmt.Errorf("failed to sync log line: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
@ -81,8 +80,8 @@ func (s *FileSink) Flush(ctx context.Context) error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
|
||||
if s.writer != nil {
|
||||
return s.writer.Flush()
|
||||
if s.file != nil {
|
||||
return s.file.Sync()
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@ -92,12 +91,6 @@ func (s *FileSink) Close() error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
|
||||
if s.writer != nil {
|
||||
if err := s.writer.Flush(); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if s.file != nil {
|
||||
return s.file.Close()
|
||||
}
|
||||
@ -122,47 +115,54 @@ func (s *FileSink) openFile() error {
|
||||
}
|
||||
|
||||
s.file = file
|
||||
s.writer = bufio.NewWriter(file)
|
||||
return nil
|
||||
}
|
||||
|
||||
// validateFilePath validates that the file path is safe and allowed.
|
||||
func validateFilePath(path string) error {
|
||||
if path == "" {
|
||||
if strings.TrimSpace(path) == "" {
|
||||
return fmt.Errorf("path cannot be empty")
|
||||
}
|
||||
|
||||
// Clean the path
|
||||
cleanPath := filepath.Clean(path)
|
||||
|
||||
// Ensure path is absolute or relative to allowed directories
|
||||
allowedPrefixes := []string{
|
||||
// Allow relative paths for testing/dev
|
||||
if !filepath.IsAbs(cleanPath) {
|
||||
return nil
|
||||
}
|
||||
|
||||
absPath, err := filepath.Abs(cleanPath)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to resolve absolute path: %w", err)
|
||||
}
|
||||
|
||||
allowedRoots := []string{
|
||||
"/var/log/logcorrelator",
|
||||
"/var/log",
|
||||
"/tmp",
|
||||
}
|
||||
|
||||
// Check if path is in allowed directories
|
||||
allowed := false
|
||||
for _, prefix := range allowedPrefixes {
|
||||
if strings.HasPrefix(cleanPath, prefix) {
|
||||
allowed = true
|
||||
break
|
||||
for _, root := range allowedRoots {
|
||||
absRoot, err := filepath.Abs(filepath.Clean(root))
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
if !allowed {
|
||||
// Allow relative paths for testing
|
||||
if !filepath.IsAbs(cleanPath) {
|
||||
rel, err := filepath.Rel(absRoot, absPath)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
if rel == "." {
|
||||
return nil
|
||||
}
|
||||
if rel == ".." {
|
||||
continue
|
||||
}
|
||||
if !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("path must be in allowed directories: %v", allowedPrefixes)
|
||||
}
|
||||
|
||||
// Check for path traversal
|
||||
if strings.Contains(cleanPath, "..") {
|
||||
return fmt.Errorf("path cannot contain '..'")
|
||||
}
|
||||
|
||||
return nil
|
||||
return fmt.Errorf("path must be under allowed directories: %v", allowedRoots)
|
||||
}
|
||||
|
||||
@ -44,6 +44,36 @@ func TestFileSink_Write(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileSink_WriteImmediatePersist_NoFlushNeeded(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
testPath := filepath.Join(tmpDir, "test.log")
|
||||
|
||||
sink, err := NewFileSink(Config{Path: testPath})
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sink: %v", err)
|
||||
}
|
||||
defer sink.Close()
|
||||
|
||||
log := domain.CorrelatedLog{
|
||||
SrcIP: "192.168.1.1",
|
||||
SrcPort: 8080,
|
||||
Correlated: true,
|
||||
}
|
||||
|
||||
if err := sink.Write(context.Background(), log); err != nil {
|
||||
t.Fatalf("failed to write: %v", err)
|
||||
}
|
||||
|
||||
// Must be visible immediately without Flush()
|
||||
data, err := os.ReadFile(testPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to read file: %v", err)
|
||||
}
|
||||
if len(data) == 0 {
|
||||
t.Error("expected data to be present immediately after Write without Flush")
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileSink_MultipleWrites(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
testPath := filepath.Join(tmpDir, "test.log")
|
||||
@ -105,7 +135,7 @@ func TestFileSink_ValidateFilePath(t *testing.T) {
|
||||
{"valid /var/log/logcorrelator", "/var/log/logcorrelator/test.log", false},
|
||||
{"valid /var/log", "/var/log/test.log", false},
|
||||
{"valid /tmp", "/tmp/test.log", false},
|
||||
{"path traversal", "/var/log/../etc/passwd", true},
|
||||
{"reject lookalike /var/logevil", "/var/logevil/test.log", true},
|
||||
{"invalid directory", "/etc/logcorrelator/test.log", true},
|
||||
{"relative path", "test.log", false}, // Allowed for testing
|
||||
}
|
||||
@ -137,9 +167,6 @@ func TestFileSink_OpenFile(t *testing.T) {
|
||||
if sink.file == nil {
|
||||
t.Error("expected file to be opened")
|
||||
}
|
||||
if sink.writer == nil {
|
||||
t.Error("expected writer to be initialized")
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileSink_WriteBeforeOpen(t *testing.T) {
|
||||
@ -183,7 +210,7 @@ func TestFileSink_FlushBeforeOpen(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestFileSink_InvalidPath(t *testing.T) {
|
||||
// Test with invalid path (path traversal)
|
||||
// Test with invalid path (outside allowed directories)
|
||||
_, err := NewFileSink(Config{Path: "/etc/../passwd"})
|
||||
if err == nil {
|
||||
t.Error("expected error for invalid path")
|
||||
|
||||
Reference in New Issue
Block a user