Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions internal/logger/file_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func handleFileLoggerError(err error, logDir, fileName string) (*FileLogger, err
// If the log directory doesn't exist and can't be created, falls back to stdout
func InitFileLogger(logDir, fileName string) error {
logger, err := initLogger(logDir, fileName, os.O_APPEND, setupFileLogger, handleFileLoggerError)
initGlobalFileLogger(logger)
initGlobalLogger(&globalLoggerMu, &globalFileLogger, logger)
return err
}

Expand Down Expand Up @@ -153,5 +153,5 @@ var logFuncs = map[LogLevel]func(string, string, ...interface{}){

// CloseGlobalLogger closes the global file logger
func CloseGlobalLogger() error {
return closeGlobalFileLogger()
return closeGlobalLogger(&globalLoggerMu, &globalFileLogger)
}
53 changes: 0 additions & 53 deletions internal/logger/global_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,56 +122,3 @@ func closeGlobalLogger[T closableLogger](mu *sync.RWMutex, logger *T) error {
}
return nil
}

// Type-specific helper functions that use the generic implementations above.
// These maintain backward compatibility with existing code.

// initGlobalFileLogger initializes the global FileLogger using the generic helper.
func initGlobalFileLogger(logger *FileLogger) {
initGlobalLogger(&globalLoggerMu, &globalFileLogger, logger)
}

// closeGlobalFileLogger closes the global FileLogger using the generic helper.
func closeGlobalFileLogger() error {
return closeGlobalLogger(&globalLoggerMu, &globalFileLogger)
}

// initGlobalJSONLLogger initializes the global JSONLLogger using the generic helper.
func initGlobalJSONLLogger(logger *JSONLLogger) {
initGlobalLogger(&globalJSONLMu, &globalJSONLLogger, logger)
}

// closeGlobalJSONLLogger closes the global JSONLLogger using the generic helper.
func closeGlobalJSONLLogger() error {
return closeGlobalLogger(&globalJSONLMu, &globalJSONLLogger)
}

// initGlobalMarkdownLogger initializes the global MarkdownLogger using the generic helper.
func initGlobalMarkdownLogger(logger *MarkdownLogger) {
initGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger, logger)
}

// closeGlobalMarkdownLogger closes the global MarkdownLogger using the generic helper.
func closeGlobalMarkdownLogger() error {
return closeGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger)
}

// initGlobalServerFileLogger initializes the global ServerFileLogger using the generic helper.
func initGlobalServerFileLogger(logger *ServerFileLogger) {
initGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger, logger)
}

// closeGlobalServerFileLogger closes the global ServerFileLogger using the generic helper.
func closeGlobalServerFileLogger() error {
return closeGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger)
}

// initGlobalToolsLogger initializes the global ToolsLogger using the generic helper.
func initGlobalToolsLogger(logger *ToolsLogger) {
initGlobalLogger(&globalToolsMu, &globalToolsLogger, logger)
}

// closeGlobalToolsLogger closes the global ToolsLogger using the generic helper.
func closeGlobalToolsLogger() error {
return closeGlobalLogger(&globalToolsMu, &globalToolsLogger)
}
4 changes: 2 additions & 2 deletions internal/logger/jsonl_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func InitJSONLLogger(logDir, fileName string) error {
// JSONLLogger has no fallback mode, so we should not initialize
// the global logger when initialization fails
if err == nil {
initGlobalJSONLLogger(logger)
initGlobalLogger(&globalJSONLMu, &globalJSONLLogger, logger)
}
return err
}
Expand Down Expand Up @@ -107,7 +107,7 @@ func (jl *JSONLLogger) logEntry(entry interface{}) error {

// CloseJSONLLogger closes the global JSONL logger
func CloseJSONLLogger() error {
return closeGlobalJSONLLogger()
return closeGlobalLogger(&globalJSONLMu, &globalJSONLLogger)
}

// LogRPCMessageJSONL logs an RPC message to the global JSONL logger
Expand Down
4 changes: 2 additions & 2 deletions internal/logger/markdown_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func handleMarkdownLoggerError(_ error, logDir, fileName string) (*MarkdownLogge
// InitMarkdownLogger initializes the global markdown logger
func InitMarkdownLogger(logDir, fileName string) error {
logger, err := initLogger(logDir, fileName, os.O_TRUNC, setupMarkdownLogger, handleMarkdownLoggerError)
initGlobalMarkdownLogger(logger)
initGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger, logger)
return err
}

Expand Down Expand Up @@ -215,5 +215,5 @@ func LogDebugMd(category, format string, args ...interface{}) {

// CloseMarkdownLogger closes the global markdown logger
func CloseMarkdownLogger() error {
return closeGlobalMarkdownLogger()
return closeGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger)
}
6 changes: 3 additions & 3 deletions internal/logger/server_file_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func InitServerFileLogger(logDir string) error {
files: make(map[string]*os.File),
useFallback: true,
}
initGlobalServerFileLogger(sfl)
initGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger, sfl)
return nil
}

Expand All @@ -49,7 +49,7 @@ func InitServerFileLogger(logDir string) error {
}

log.Printf("Initialized per-serverID logging in directory: %s", logDir)
initGlobalServerFileLogger(sfl)
initGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger, sfl)
return nil
}

Expand Down Expand Up @@ -177,5 +177,5 @@ func LogDebugWithServer(serverID, category, format string, args ...interface{})

// CloseServerFileLogger closes the global server file logger
func CloseServerFileLogger() error {
return closeGlobalServerFileLogger()
return closeGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger)
}
4 changes: 2 additions & 2 deletions internal/logger/tools_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func handleToolsLoggerError(err error, logDir, fileName string) (*ToolsLogger, e
// If the log directory doesn't exist and can't be created, falls back to no-op
func InitToolsLogger(logDir, fileName string) error {
logger, err := initLogger(logDir, fileName, os.O_TRUNC, setupToolsLogger, handleToolsLoggerError)
initGlobalToolsLogger(logger)
initGlobalLogger(&globalToolsMu, &globalToolsLogger, logger)
return err
}

Expand Down Expand Up @@ -149,5 +149,5 @@ func LogToolsForServer(serverID string, tools []ToolInfo) {

// CloseToolsLogger closes the global tools logger
func CloseToolsLogger() error {
return closeGlobalToolsLogger()
return closeGlobalLogger(&globalToolsMu, &globalToolsLogger)
}
8 changes: 2 additions & 6 deletions internal/server/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,13 @@ func authMiddleware(apiKey string, next http.HandlerFunc) http.HandlerFunc {

if authHeader == "" {
// Spec 7.1: Missing token returns 401
logger.LogErrorMd("auth", "Authentication failed: missing Authorization header, remote=%s, path=%s", r.RemoteAddr, r.URL.Path)
logRuntimeError("authentication_failed", "missing_auth_header", r, nil)
writeErrorResponse(w, http.StatusUnauthorized, "unauthorized", "missing Authorization header")
rejectRequest(w, r, http.StatusUnauthorized, "unauthorized", "missing Authorization header", "auth", "authentication_failed", "missing_auth_header")
return
}

// Spec 7.1: Authorization header must contain API key directly (not Bearer scheme)
if authHeader != apiKey {
logger.LogErrorMd("auth", "Authentication failed: invalid API key, remote=%s, path=%s", r.RemoteAddr, r.URL.Path)
logRuntimeError("authentication_failed", "invalid_api_key", r, nil)
writeErrorResponse(w, http.StatusUnauthorized, "unauthorized", "invalid API key")
rejectRequest(w, r, http.StatusUnauthorized, "unauthorized", "invalid API key", "auth", "authentication_failed", "invalid_api_key")
return
Comment on lines 47 to 56
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

These rejectRequest calls now log runtime errors as error_type=missing_auth_header / invalid_api_key with detail set to the human message, whereas the prior behavior was error_type=authentication_failed with detail set to the specific reason token. If downstream tooling/monitoring expects authentication_failed as the type, this is a behavior change; pass authentication_failed as the runtime error type and keep missing_auth_header/invalid_api_key as the detail (or adjust rejectRequest signature accordingly).

See below for a potential fix:

			rejectRequest(w, r, http.StatusUnauthorized, "unauthorized", "missing Authorization header", "auth", "authentication_failed")
			return
		}

		// Spec 7.1: Authorization header must contain API key directly (not Bearer scheme)
		if authHeader != apiKey {
			rejectRequest(w, r, http.StatusUnauthorized, "unauthorized", "invalid API key", "auth", "authentication_failed")

Copilot uses AI. Check for mistakes.
}

Expand Down
15 changes: 15 additions & 0 deletions internal/server/http_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,21 @@ func writeErrorResponse(w http.ResponseWriter, statusCode int, code, message str
})
}

// rejectRequest logs a structured error, records a runtime error, and writes an
// HTTP error response. This consolidates the 3-step rejection pattern that was
// previously duplicated across auth and handler code paths.
//
// Parameters:
// - logCategory: category for the structured log (e.g. "auth")
// - runtimeErrType: error_type field for runtime error log (e.g. "authentication_failed")
// - runtimeDetail: detail field for runtime error log (e.g. "missing_auth_header")
// - msg: human-readable message sent back in the HTTP response
func rejectRequest(w http.ResponseWriter, r *http.Request, status int, code, msg, logCategory, runtimeErrType, runtimeDetail string) {
logger.LogErrorMd(logCategory, "Request rejected: %s, remote=%s, path=%s", msg, r.RemoteAddr, r.URL.Path)
logRuntimeError(runtimeErrType, runtimeDetail, r, nil)
writeErrorResponse(w, status, code, msg)
}

// withResponseLogging wraps an http.Handler to log response bodies
func withResponseLogging(handler http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down
Loading