Skip to content

fix: Request Id was empty whehn audit logs were disabled +remove redundant request ID logic from audit log middleware #101

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
SantiagoDePolonia merged 1 commit into from
Feb 25, 2026
Merged

fix: Request Id was empty whehn audit logs were disabled +remove redundant request ID logic from audit log middleware #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 1 addition & 10 deletions internal/auditlog/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,8 @@ func Middleware(logger LoggerInterface) echo.MiddlewareFunc {
start := time.Now()
req := c.Request()

// Generate request ID if not present
// Read request ID (always set by the request ID middleware in http.go)
requestID := req.Header.Get("X-Request-ID")
if requestID == "" {
requestID = uuid.NewString()
}

// Set request ID in both request and response headers
// Request header: for internal components to read consistently
// Response header: for clients to receive in the response
req.Header.Set("X-Request-ID", requestID)
c.Response().Header().Set("X-Request-ID", requestID)

// Create initial log entry
entry := &LogEntry{
Expand Down
15 changes: 15 additions & 0 deletions internal/server/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path"
"strings"

"github.com/google/uuid"
"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
"github.com/prometheus/client_golang/prometheus/promhttp"
Expand Down Expand Up @@ -132,6 +133,20 @@ func New(provider core.RoutableProvider, cfg *Config) *Server {
}
e.Use(middleware.BodyLimit(bodySizeLimit))

// Request ID middleware (always active — ensures every request has a unique ID
// for usage tracking, audit logging, and response correlation)
e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
return func(c echo.Context) error {
id := c.Request().Header.Get("X-Request-ID")
if id == "" {
id = uuid.NewString()
c.Request().Header.Set("X-Request-ID", id)
}
c.Response().Header().Set("X-Request-ID", id)
return next(c)
}
})

// Audit logging middleware (before authentication to capture all requests)
if cfg != nil && cfg.AuditLogger != nil && cfg.AuditLogger.Config().Enabled {
e.Use(auditlog.Middleware(cfg.AuditLogger))
Expand Down
41 changes: 41 additions & 0 deletions internal/server/http_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,47 @@ import (
_ "gomodel/cmd/gomodel/docs"
)

func TestRequestIDMiddleware(t *testing.T) {
mock := &mockProvider{}
srv := New(mock, nil)

t.Run("generates request ID when missing", func(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/health", nil)
rec := httptest.NewRecorder()

srv.ServeHTTP(rec, req)

got := rec.Header().Get("X-Request-ID")
if got == "" {
t.Fatal("expected X-Request-ID in response header, got empty")
}
// Validate UUID format (8-4-4-4-12 hex digits)
if len(got) != 36 {
t.Errorf("expected UUID (36 chars), got %q (%d chars)", got, len(got))
}
Comment on lines +29 to +32

@coderabbitai coderabbitai Bot Feb 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

UUID format validation is weaker than the comment implies.

The comment on line 29 states "Validate UUID format (8-4-4-4-12 hex digits)" but the check only validates the string length (36). Any 36-character string would pass — e.g., a non-UUID. Consider using uuid.Parse to assert a well-formed UUID v4.

♻️ Proposed stronger validation 
+import "github.com/google/uuid"
 
-		// Validate UUID format (8-4-4-4-12 hex digits)
-		if len(got) != 36 {
-			t.Errorf("expected UUID (36 chars), got %q (%d chars)", got, len(got))
-		}
+		// Validate well-formed UUID
+		if _, err := uuid.Parse(got); err != nil {
+			t.Errorf("expected valid UUID, got %q: %v", got, err)
+		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Validate UUID format (8-4-4-4-12 hex digits)
if len(got) != 36 {
t.Errorf("expected UUID (36 chars), got %q (%d chars)", got, len(got))
}
// Validate well-formed UUID
if _, err := uuid.Parse(got); err != nil {
t.Errorf("expected valid UUID, got %q: %v", got, err)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/server/http_test.go` around lines 29 - 32, The current check only
verifies len(got)==36 which allows non-UUID strings; replace it by parsing the
value in variable got using a UUID parser (e.g., uuid.Parse from
github.com/google/uuid or uuid.Parse from github.com/gofrs/uuid) and assert the
parse returns no error (and optionally assert the parsed UUID's Version()==4 if
you want to enforce v4). Update the test around the existing validation (the
block that checks len(got) and references got) to call uuid.Parse(got), fail the
test on parse error, and remove or keep a shorter length check only as a
fallback.

})

t.Run("preserves existing request ID", func(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/health", nil)
req.Header.Set("X-Request-ID", "my-custom-id")
rec := httptest.NewRecorder()

srv.ServeHTTP(rec, req)

// Request header must not be overwritten
got := req.Header.Get("X-Request-ID")
if got != "my-custom-id" {
t.Errorf("expected request header to be preserved as %q, got %q", "my-custom-id", got)
}

// Response header must echo the client-provided ID back
respID := rec.Header().Get("X-Request-ID")
if respID != "my-custom-id" {
t.Errorf("expected response header X-Request-ID to be %q, got %q", "my-custom-id", respID)
}
})
}

func TestMetricsEndpoint(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading