feat: implement Request ID middleware for log correlation

[3rabiii]

Description

Addresses the High Severity feature gap regarding the lack of Request ID/Correlation ID tracking.

This PR introduces a middleware that ensures every request has a unique identifier, allowing for better log correlation and debugging across distributed components.

Changes:

• New Middleware: Added internal/restapi/request_id_middleware.go to:
  • Check for an existing X-Request-ID header.
  • Generate a new UUID if the header is missing.
  • Inject the ID into the request context.
  • Return the ID in the response headers.
• Server Wiring: Updated cmd/api/main.go to wrap the API handler with this new middleware.
• Testing: Added unit tests in internal/restapi/request_id_middleware_test.go to verify behavior for both existing and missing headers.

Verification

@aaronbrethorst
closes : #369

[aaronbrethorst]

Hey Adel, appreciate you tackling request tracing — having a correlation ID flowing through the system is genuinely important for production debugging. The middleware itself is clean and the tests cover the two core paths (generate vs. preserve). However, there are a few things we need to sort out before this can merge:

Critical

1. A 22MB compiled binary was committed to the repo (api at the repo root): This is a Linux ELF executable that was accidentally included in the commit. Binary artifacts should never be checked into source control — they bloat the repo permanently (even after deletion, they remain in git history). Please:
   • Remove the file from the commit: git rm api
   • Add api to .gitignore (the existing entry /cmd/api/api only covers builds in that subdirectory, not the repo root)
   • Force-push the cleaned branch so the binary doesn't persist in git history

Important

2. The request ID is never included in log output (request_logging_middleware.go): The stated purpose of this feature is "log correlation," but the request logging middleware doesn't read the request ID from the context. Currently, the only visible effect is the X-Request-ID response header — no log entry anywhere includes the ID. For this feature to deliver on its promise, the request logging middleware needs to include the request ID:

   // In request_logging_middleware.go, inside the handler func:
   reqID, _ := r.Context().Value(restapi.RequestIDKey).(string)
   
   logging.LogHTTPRequest(logger,
       r.Method,
       r.URL.Path,
       wrapped.statusCode,
       float64(duration.Nanoseconds())/1e6,
       slog.String("request_id", reqID),   // <-- add this
       slog.String("user_agent", r.Header.Get("User-Agent")),
       slog.String("component", "http_server"))

   Without this, the feature is incomplete — we're generating IDs nobody can see.

3. No validation of incoming X-Request-ID header (request_id_middleware.go:16): The middleware blindly trusts whatever value a client sends. An attacker could pass a multi-megabyte string or one containing non-printable characters, which then gets stored in the context and potentially logged. Validate the input:

   reqID := r.Header.Get("X-Request-ID")
   if reqID == "" || len(reqID) > 128 || !isValidRequestID(reqID) {
       reqID = uuid.NewString()
   }

   Where isValidRequestID checks for alphanumeric characters, hyphens, underscores, dots, and colons (the typical character set for trace IDs). Add a test case for an oversized or malformed X-Request-ID being replaced.

4. go mod tidy not run: github.com/google/uuid is now a direct import in request_id_middleware.go, but go.mod still lists it as // indirect. Run go mod tidy to fix the categorization.

5. Wire the middleware inside CreateServer in app.go, not in main.go (main.go:112-114): Every other middleware is composed inside CreateServer — that's where the middleware chain lives. Bolting this on afterwards in main() bypasses the established pattern and makes the chain harder to reason about. Move it into app.go as the outermost layer (wrapping the request logging middleware):

   // In app.go CreateServer()
   handler := restapi.RequestIDMiddleware(requestLogMiddleware(secureHandler))

   This also eliminates the unnecessary if srv.Handler != nil guard, since CreateServer always sets the handler.

Fit and Finish

6. Add a test for the generated ID format (request_id_middleware_test.go): The "should generate request ID if missing" test asserts NotEmpty but doesn't verify the ID is a valid UUID. Add assert.Regexp(t, ^[0-9a-f-]{36}$, respID) to catch future regressions if the generation logic changes.

Recommended Action

1. Remove the binary and add to .gitignore (item 1)
2. Add request ID to log output (item 2)
3. Validate incoming X-Request-ID (item 3)
4. Run go mod tidy (item 4)
5. Move middleware wiring to app.go (item 5)
6. Squash into a single clean commit after changes

Thanks again, and I look forward to merging this change.

[3rabiii]

@aaronbrethorst
Ready for review!

[aaronbrethorst]

Hey Adel, excellent work addressing all six items from the previous review — every single one is properly resolved. The binary is gone, validation is solid, the middleware is correctly wired inside CreateServer, request IDs flow through to log output, go.mod is tidy, and the UUID format assertion is in the tests. The commit is clean and well-organized.

I have a few non-blocking suggestions that would strengthen this feature, but none of them are required for merge:

Suggestions

1. Add a GetRequestID(ctx) helper function (request_id_middleware.go): The contextKey type is unexported, which means code in other packages (e.g., logging, webui, or future handlers) can't easily read the request ID from context without importing restapi. Adding a simple accessor follows the pattern already established by logging.FromContext():

   func GetRequestID(ctx context.Context) string {
       if id, ok := ctx.Value(RequestIDKey).(string); ok {
           return id
       }
       return ""
   }

   This isn't needed today since the only consumer (request_logging_middleware.go) is in the same package, but it would make the feature more accessible as the codebase grows.

2. Add an integration test for request ID appearing in log output (request_id_middleware_test.go or request_logging_test.go): The whole purpose of this feature is log correlation, but no test currently verifies that composing RequestIDMiddleware with NewRequestLoggingMiddleware actually produces "request_id":"..." in the log output. A quick integration test would guard against future regressions in context key wiring.

3. Add a boundary test for exactly 128 characters (request_id_middleware_test.go): The tests cover 129 characters (rejected) but not 128 characters (accepted). A boundary test would catch an off-by-one regression if someone changes > to >=.

There's nothing left to change — this PR is ready to merge. Clean middleware, thorough validation, proper test coverage, and all prior feedback addressed. Nice work.