From 89d68962b1d01158866176bb528c287b00ff3554 Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 5 Dec 2025 10:01:05 +0800 Subject: [PATCH 1/3] fix(amp): filter amp request logging to only provider endpoint --- internal/api/middleware/request_logging.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/api/middleware/request_logging.go b/internal/api/middleware/request_logging.go index 0bef6752..8f29e1a1 100644 --- a/internal/api/middleware/request_logging.go +++ b/internal/api/middleware/request_logging.go @@ -112,5 +112,10 @@ func shouldLogRequest(path string) bool { if strings.HasPrefix(path, "/v0/management") || strings.HasPrefix(path, "/management") { return false } + + if strings.HasPrefix(path, "/api") { + return strings.HasPrefix(path, "/api/provider") + } + return true } From acdfb3bcebf2e230f1fe3e4f6136e97f53dce3b9 Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 5 Dec 2025 10:46:01 +0800 Subject: [PATCH 2/3] feat(amp): add root-level /threads routes for CLI compatibility --- internal/api/modules/amp/routes.go | 1 + internal/api/modules/amp/routes_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/api/modules/amp/routes.go b/internal/api/modules/amp/routes.go index b986a53a..9cb4ab8b 100644 --- a/internal/api/modules/amp/routes.go +++ b/internal/api/modules/amp/routes.go @@ -128,6 +128,7 @@ func (m *AmpModule) registerManagementRoutes(engine *gin.Engine, baseHandler *ha // Root-level routes that AMP CLI expects without /api prefix // These need the same security middleware as the /api/* routes (dynamic for hot-reload) rootMiddleware := []gin.HandlerFunc{noCORSMiddleware(), m.localhostOnlyMiddleware()} + engine.GET("/threads/*path", append(rootMiddleware, proxyHandler)...) engine.GET("/threads.rss", append(rootMiddleware, proxyHandler)...) // Root-level auth routes for CLI login flow diff --git a/internal/api/modules/amp/routes_test.go b/internal/api/modules/amp/routes_test.go index a40852c0..67b39190 100644 --- a/internal/api/modules/amp/routes_test.go +++ b/internal/api/modules/amp/routes_test.go @@ -47,6 +47,7 @@ func TestRegisterManagementRoutes(t *testing.T) { {"/api/meta", http.MethodGet}, {"/api/telemetry", http.MethodGet}, {"/api/threads", http.MethodGet}, + {"/threads/", http.MethodGet}, {"/threads.rss", http.MethodGet}, // Root-level route (no /api prefix) {"/api/otel", http.MethodGet}, {"/api/tab", http.MethodGet}, From caa48e7c6f77bcb4112f8fd576e6e5a92388e9c3 Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 5 Dec 2025 21:09:53 +0800 Subject: [PATCH 3/3] fix(amp): improve proxy state management and request logging behavior --- internal/api/modules/amp/amp.go | 93 +++++++++---------- internal/api/modules/amp/fallback_handlers.go | 8 +- internal/api/modules/amp/routes.go | 25 ++++- internal/logging/gin_logger.go | 27 ++++++ 4 files changed, 93 insertions(+), 60 deletions(-) diff --git a/internal/api/modules/amp/amp.go b/internal/api/modules/amp/amp.go index 9b256ac5..dabb7404 100644 --- a/internal/api/modules/amp/amp.go +++ b/internal/api/modules/amp/amp.go @@ -126,6 +126,9 @@ func (m *AmpModule) Register(ctx modules.Context) error { // Always register provider aliases - these work without an upstream m.registerProviderAliases(ctx.Engine, ctx.BaseHandler, auth) + // Register management proxy routes once; middleware will gate access when upstream is unavailable. + m.registerManagementRoutes(ctx.Engine, ctx.BaseHandler) + // If no upstream URL, skip proxy routes but provider aliases are still available if upstreamURL == "" { log.Debug("amp upstream proxy disabled (no upstream URL configured)") @@ -134,27 +137,11 @@ func (m *AmpModule) Register(ctx modules.Context) error { return } - // Create secret source with precedence: config > env > file - // Cache secrets for 5 minutes to reduce file I/O - if m.secretSource == nil { - m.secretSource = NewMultiSourceSecret(settings.UpstreamAPIKey, 0 /* default 5min */) - } - - // Create reverse proxy with gzip handling via ModifyResponse - proxy, err := createReverseProxy(upstreamURL, m.secretSource) - if err != nil { + if err := m.enableUpstreamProxy(upstreamURL, &settings); err != nil { regErr = fmt.Errorf("failed to create amp proxy: %w", err) return } - m.setProxy(proxy) - m.enabled = true - - // Register management proxy routes (requires upstream) - // Uses dynamic middleware that checks m.IsRestrictedToLocalhost() for hot-reload support - m.registerManagementRoutes(ctx.Engine, ctx.BaseHandler) - - log.Infof("amp upstream proxy enabled for: %s", upstreamURL) log.Debug("amp provider alias routes registered") }) @@ -188,18 +175,30 @@ func (m *AmpModule) OnConfigUpdated(cfg *config.Config) error { oldSettings := m.lastConfig m.configMu.RUnlock() - // Track what changed for logging - var changes []string + if oldSettings != nil && oldSettings.RestrictManagementToLocalhost != newSettings.RestrictManagementToLocalhost { + m.setRestrictToLocalhost(newSettings.RestrictManagementToLocalhost) + if !newSettings.RestrictManagementToLocalhost { + log.Warnf("amp management routes now accessible from any IP - this is insecure!") + } + } + + newUpstreamURL := strings.TrimSpace(newSettings.UpstreamURL) + oldUpstreamURL := "" + if oldSettings != nil { + oldUpstreamURL = strings.TrimSpace(oldSettings.UpstreamURL) + } + + if !m.enabled && newUpstreamURL != "" { + if err := m.enableUpstreamProxy(newUpstreamURL, &newSettings); err != nil { + log.Errorf("amp config: failed to enable upstream proxy for %s: %v", newUpstreamURL, err) + } + } // Check model mappings change modelMappingsChanged := m.hasModelMappingsChanged(oldSettings, &newSettings) if modelMappingsChanged { if m.modelMapper != nil { m.modelMapper.UpdateMappings(newSettings.ModelMappings) - changes = append(changes, "model-mappings") - if m.enabled { - log.Infof("amp config partial reload: model mappings updated (%d entries)", len(newSettings.ModelMappings)) - } } else if m.enabled { log.Warnf("amp model mapper not initialized, skipping model mapping update") } @@ -207,25 +206,16 @@ func (m *AmpModule) OnConfigUpdated(cfg *config.Config) error { if m.enabled { // Check upstream URL change - now supports hot-reload - newUpstreamURL := strings.TrimSpace(newSettings.UpstreamURL) - oldUpstreamURL := "" - if oldSettings != nil { - oldUpstreamURL = strings.TrimSpace(oldSettings.UpstreamURL) - } - if newUpstreamURL == "" && oldUpstreamURL != "" { - log.Warn("amp upstream URL removed from config, proxy has been disabled") m.setProxy(nil) - changes = append(changes, "upstream-url(disabled)") - } else if newUpstreamURL != oldUpstreamURL && newUpstreamURL != "" { + m.enabled = false + } else if oldUpstreamURL != "" && newUpstreamURL != oldUpstreamURL && newUpstreamURL != "" { // Recreate proxy with new URL proxy, err := createReverseProxy(newUpstreamURL, m.secretSource) if err != nil { log.Errorf("amp config: failed to create proxy for new upstream URL %s: %v", newUpstreamURL, err) } else { m.setProxy(proxy) - changes = append(changes, "upstream-url") - log.Infof("amp config partial reload: upstream URL updated (%s -> %s)", oldUpstreamURL, newUpstreamURL) } } @@ -236,22 +226,10 @@ func (m *AmpModule) OnConfigUpdated(cfg *config.Config) error { if ms, ok := m.secretSource.(*MultiSourceSecret); ok { ms.UpdateExplicitKey(newSettings.UpstreamAPIKey) ms.InvalidateCache() - changes = append(changes, "upstream-api-key") - log.Debug("amp config partial reload: secret cache invalidated") } } } - // Check restrict-management-to-localhost change - now supports hot-reload - if oldSettings != nil && oldSettings.RestrictManagementToLocalhost != newSettings.RestrictManagementToLocalhost { - m.setRestrictToLocalhost(newSettings.RestrictManagementToLocalhost) - changes = append(changes, "restrict-management-to-localhost") - if newSettings.RestrictManagementToLocalhost { - log.Infof("amp config partial reload: management routes now restricted to localhost") - } else { - log.Warnf("amp config partial reload: management routes now accessible from any IP - this is insecure!") - } - } } // Store current config for next comparison @@ -260,13 +238,26 @@ func (m *AmpModule) OnConfigUpdated(cfg *config.Config) error { m.lastConfig = &settingsCopy m.configMu.Unlock() - // Log summary if any changes detected - if len(changes) > 0 { - log.Debugf("amp config partial reload completed: %v", changes) - } else { - log.Debug("amp config checked: no changes detected") + return nil +} + +func (m *AmpModule) enableUpstreamProxy(upstreamURL string, settings *config.AmpCode) error { + if m.secretSource == nil { + m.secretSource = NewMultiSourceSecret(settings.UpstreamAPIKey, 0 /* default 5min */) + } else if ms, ok := m.secretSource.(*MultiSourceSecret); ok { + ms.UpdateExplicitKey(settings.UpstreamAPIKey) + ms.InvalidateCache() } + proxy, err := createReverseProxy(upstreamURL, m.secretSource) + if err != nil { + return err + } + + m.setProxy(proxy) + m.enabled = true + + log.Infof("amp upstream proxy enabled for: %s", upstreamURL) return nil } diff --git a/internal/api/modules/amp/fallback_handlers.go b/internal/api/modules/amp/fallback_handlers.go index 69d8ba74..59c83e22 100644 --- a/internal/api/modules/amp/fallback_handlers.go +++ b/internal/api/modules/amp/fallback_handlers.go @@ -48,25 +48,25 @@ func logAmpRouting(routeType AmpRouteType, requestedModel, resolvedModel, provid case RouteTypeLocalProvider: fields["cost"] = "free" fields["source"] = "local_oauth" - log.WithFields(fields).Infof("[amp] using local provider for model: %s", requestedModel) + log.WithFields(fields).Debugf("amp using local provider for model: %s", requestedModel) case RouteTypeModelMapping: fields["cost"] = "free" fields["source"] = "local_oauth" fields["mapping"] = requestedModel + " -> " + resolvedModel - log.WithFields(fields).Infof("[amp] model mapped: %s -> %s", requestedModel, resolvedModel) + // model mapping already logged in mapper; avoid duplicate here case RouteTypeAmpCredits: fields["cost"] = "amp_credits" fields["source"] = "ampcode.com" fields["model_id"] = requestedModel // Explicit model_id for easy config reference - log.WithFields(fields).Warnf("[amp] forwarding to ampcode.com (uses amp credits) - model_id: %s | To use local proxy, add to config: amp-model-mappings: [{from: \"%s\", to: \"\"}]", requestedModel, requestedModel) + log.WithFields(fields).Warnf("forwarding to ampcode.com (uses amp credits) - model_id: %s | To use local proxy, add to config: amp-model-mappings: [{from: \"%s\", to: \"\"}]", requestedModel, requestedModel) case RouteTypeNoProvider: fields["cost"] = "none" fields["source"] = "error" fields["model_id"] = requestedModel // Explicit model_id for easy config reference - log.WithFields(fields).Warnf("[amp] no provider available for model_id: %s", requestedModel) + log.WithFields(fields).Warnf("no provider available for model_id: %s", requestedModel) } } diff --git a/internal/api/modules/amp/routes.go b/internal/api/modules/amp/routes.go index 9cb4ab8b..647561b6 100644 --- a/internal/api/modules/amp/routes.go +++ b/internal/api/modules/amp/routes.go @@ -2,10 +2,12 @@ package amp import ( "net" + "net/http" "net/http/httputil" "strings" "github.com/gin-gonic/gin" + "github.com/router-for-me/CLIProxyAPI/v6/internal/logging" "github.com/router-for-me/CLIProxyAPI/v6/internal/util" "github.com/router-for-me/CLIProxyAPI/v6/sdk/api/handlers" "github.com/router-for-me/CLIProxyAPI/v6/sdk/api/handlers/claude" @@ -78,6 +80,21 @@ func noCORSMiddleware() gin.HandlerFunc { } } +// managementAvailabilityMiddleware short-circuits management routes when the upstream +// proxy is disabled, preventing noisy localhost warnings and accidental exposure. +func (m *AmpModule) managementAvailabilityMiddleware() gin.HandlerFunc { + return func(c *gin.Context) { + if m.getProxy() == nil { + logging.SkipGinRequestLogging(c) + c.AbortWithStatusJSON(http.StatusServiceUnavailable, gin.H{ + "error": "amp upstream proxy not available", + }) + return + } + c.Next() + } +} + // registerManagementRoutes registers Amp management proxy routes // These routes proxy through to the Amp control plane for OAuth, user management, etc. // Uses dynamic middleware and proxy getter for hot-reload support. @@ -85,14 +102,12 @@ func (m *AmpModule) registerManagementRoutes(engine *gin.Engine, baseHandler *ha ampAPI := engine.Group("/api") // Always disable CORS for management routes to prevent browser-based attacks - ampAPI.Use(noCORSMiddleware()) + ampAPI.Use(m.managementAvailabilityMiddleware(), noCORSMiddleware()) // Apply dynamic localhost-only restriction (hot-reloadable via m.IsRestrictedToLocalhost()) ampAPI.Use(m.localhostOnlyMiddleware()) - if m.IsRestrictedToLocalhost() { - log.Info("amp management routes restricted to localhost only (CORS disabled)") - } else { + if !m.IsRestrictedToLocalhost() { log.Warn("amp management routes are NOT restricted to localhost - this is insecure!") } @@ -127,7 +142,7 @@ func (m *AmpModule) registerManagementRoutes(engine *gin.Engine, baseHandler *ha // Root-level routes that AMP CLI expects without /api prefix // These need the same security middleware as the /api/* routes (dynamic for hot-reload) - rootMiddleware := []gin.HandlerFunc{noCORSMiddleware(), m.localhostOnlyMiddleware()} + rootMiddleware := []gin.HandlerFunc{m.managementAvailabilityMiddleware(), noCORSMiddleware(), m.localhostOnlyMiddleware()} engine.GET("/threads/*path", append(rootMiddleware, proxyHandler)...) engine.GET("/threads.rss", append(rootMiddleware, proxyHandler)...) diff --git a/internal/logging/gin_logger.go b/internal/logging/gin_logger.go index 2933a0bb..a4e020b1 100644 --- a/internal/logging/gin_logger.go +++ b/internal/logging/gin_logger.go @@ -14,6 +14,8 @@ import ( log "github.com/sirupsen/logrus" ) +const skipGinLogKey = "__gin_skip_request_logging__" + // GinLogrusLogger returns a Gin middleware handler that logs HTTP requests and responses // using logrus. It captures request details including method, path, status code, latency, // client IP, and any error messages, formatting them in a Gin-style log format. @@ -28,6 +30,10 @@ func GinLogrusLogger() gin.HandlerFunc { c.Next() + if shouldSkipGinRequestLogging(c) { + return + } + if raw != "" { path = path + "?" + raw } @@ -77,3 +83,24 @@ func GinLogrusRecovery() gin.HandlerFunc { c.AbortWithStatus(http.StatusInternalServerError) }) } + +// SkipGinRequestLogging marks the provided Gin context so that GinLogrusLogger +// will skip emitting a log line for the associated request. +func SkipGinRequestLogging(c *gin.Context) { + if c == nil { + return + } + c.Set(skipGinLogKey, true) +} + +func shouldSkipGinRequestLogging(c *gin.Context) bool { + if c == nil { + return false + } + val, exists := c.Get(skipGinLogKey) + if !exists { + return false + } + flag, ok := val.(bool) + return ok && flag +}