Skip to content
Open
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
1 change: 1 addition & 0 deletions gateway/api_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,7 @@

muxer := &proxyMux{
track404Logs: gwConf.Track404Logs,
gw: gw,

Check warning on line 1068 in gateway/api_loader.go

View check run for this annotation

probelabs / Visor: architecture

architecture Issue

The `proxyMux` now holds a reference to the entire `Gateway` object. This increases coupling between the routing component (`proxyMux`) and the main gateway controller, granting it access to more state than necessary for its task of recording 404 analytics.
Raw output
For a more loosely coupled design, consider passing a more focused dependency to `proxyMux` instead of the entire `Gateway` object. An interface exposing only the necessary methods (e.g., `RecordHit`, `GetAnalyticsConfig`) would follow the Principle of Least Privilege and reduce coupling, making `proxyMux` easier to test and reason about in isolation.
}
router := mux.NewRouter()
router.NotFoundHandler = http.HandlerFunc(muxer.handle404)
Expand Down
64 changes: 64 additions & 0 deletions gateway/proxy_muxer.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"github.com/TykTechnologies/again"
"github.com/TykTechnologies/tyk/config"
"github.com/TykTechnologies/tyk/internal/httputil"
"github.com/TykTechnologies/tyk/request"
"github.com/TykTechnologies/tyk/tcp"

"github.com/gorilla/mux"
Expand Down Expand Up @@ -134,6 +135,7 @@
proxies []*proxy
again again.Again
track404Logs bool
gw *Gateway
}

func (m *proxyMux) getProxy(listenPort int, conf config.Config) *proxy {
Expand Down Expand Up @@ -208,9 +210,71 @@

func (m *proxyMux) handle404(w http.ResponseWriter, r *http.Request) {
if m.track404Logs {
// Existing logging functionality
requestMeta := fmt.Sprintf("%s %s %s", r.Method, r.URL.Path, r.Proto)
log.WithField("request", requestMeta).WithField("origin", r.RemoteAddr).
Error(http.StatusText(http.StatusNotFound))

// NEW: Also create analytics record if gateway and analytics are available

Check notice on line 218 in gateway/proxy_muxer.go

View check run for this annotation

probelabs / Visor: security

security Issue

The client IP address, used for the `ignored_ips` check, is determined using `request.RealIP`. This function trusts HTTP headers like `X-Real-IP` and `X-Forwarded-For`. If Tyk is deployed behind a reverse proxy or load balancer that does not properly sanitize these headers, a client could spoof their IP address. This would allow them to bypass the `ignored_ips` filter, causing their requests to be logged to analytics when they should be ignored.
Raw output
Add documentation to highlight that for the `ignored_ips` feature to be effective for 404 logging, the networking infrastructure in front of Tyk must be configured to correctly set or strip `X-Forwarded-For` and `X-Real-IP` headers.
if m.gw != nil && m.gw.Analytics.Store != nil {
clientIP := request.RealIP(r)

Check warning on line 221 in gateway/proxy_muxer.go

View check run for this annotation

probelabs / Visor: performance

performance Issue

The `StoreAnalytics` function, which is called for every 404 request when this feature is enabled, performs a linear scan over the `AnalyticsConfig.IgnoredIPs` slice. This operation has a time complexity of O(n), where n is the number of IPs in the ignore list. For configurations with a large number of ignored IPs, this can introduce latency and CPU overhead on a potentially hot path (handling unmatched requests).
Raw output
To optimize the IP lookup, convert the `IgnoredIPs` slice into a map (e.g., `map[string]struct{}`) when the gateway configuration is loaded. This will change the lookup complexity to O(1), providing a significant performance improvement for users with large IP ignore lists.
// Check if analytics should be recorded (respects enable_analytics and ignored_ips)
gwConfig := m.gw.GetConfig()
if gwConfig.StoreAnalytics(clientIP) {
t := time.Now()

host := r.URL.Host
if host == "" {
host = r.Host
}

Check warning on line 230 in gateway/proxy_muxer.go

View check run for this annotation

probelabs / Visor: security

security Issue

The analytics record for 404 requests includes the raw URL query string (`r.URL.RawQuery`). If sensitive information such as API keys, session tokens, or personally identifiable information (PII) is passed in the URL, it will be persisted in the analytics store. This could lead to sensitive data exposure in logs and analytics systems.
Raw output
Implement a scrubbing mechanism to redact sensitive query parameters from the `RawRequest` field before recording the analytics data. This could be based on a configurable blocklist of parameter names (e.g., 'token', 'password', 'apikey').

record := analytics.AnalyticsRecord{
Method: r.Method,
Host: host,
Path: r.URL.Path,
RawPath: r.URL.Path,
RawRequest: r.URL.RawQuery,
ContentLength: r.ContentLength,
UserAgent: r.Header.Get("User-Agent"),
Day: t.Day(),
Month: t.Month(),
Year: t.Year(),
Hour: t.Hour(),
ResponseCode: http.StatusNotFound,
APIKey: "",
TimeStamp: t,
APIVersion: "",
APIName: "",
APIID: "",
OrgID: "",
OauthID: "",
RequestTime: 0,
IPAddress: clientIP,
Geo: analytics.GeoData{},
Network: analytics.NetworkStats{},
Latency: analytics.Latency{
Total: 0,
Upstream: 0,
},
Tags: []string{"404", "path-not-found"},
Alias: "",
TrackPath: false,
ExpireAt: t.Add(time.Duration(gwConfig.AnalyticsConfig.StorageExpirationTime) * time.Second),
}

Check failure on line 264 in gateway/proxy_muxer.go

View check run for this annotation

probelabs / Visor: architecture

architecture Issue

The manual creation of `analytics.AnalyticsRecord` duplicates a significant amount of logic for populating the record from an `http.Request`, which is also present in `gateway/handler_success.go` in the `RecordHit` method. This large block of code represents a missed opportunity for abstraction and reuse, leading to maintainability issues.
Raw output
To improve reusability and reduce code duplication, create a helper function that constructs a base `AnalyticsRecord` from an `*http.Request`. This function would populate common fields (e.g., `Method`, `Host`, `Path`, `UserAgent`, `TimeStamp`). Both `handle404` and `SuccessHandler.RecordHit` could then call this helper and add their context-specific data (e.g., `ResponseCode`, `Tags`, `APIID`). This would centralize the record creation logic, making it more maintainable and consistent.

// Enable GeoIP if configured
if gwConfig.AnalyticsConfig.EnableGeoIP && m.gw.Analytics.GeoIPDB != nil {
record.GetGeo(clientIP, m.gw.Analytics.GeoIPDB)
}

Check warning on line 269 in gateway/proxy_muxer.go

View check run for this annotation

probelabs / Visor: security

security Issue

This feature introduces analytics recording for every 404 request when `track_404_logs` is enabled. Since these requests do not match any defined API, they are not subject to API-level rate limiting or quota management. A malicious actor could launch a high-volume denial-of-service attack by sending requests to random, non-existent endpoints, potentially overwhelming the gateway's analytics buffer, Redis, and the Tyk Pump infrastructure.
Raw output
Consider introducing a global rate limit specifically for logging 404 errors to prevent the analytics system from being flooded by unauthenticated traffic.

Check warning on line 269 in gateway/proxy_muxer.go

View check run for this annotation

probelabs / Visor: connectivity

performance Issue

A high volume of 404 requests will trigger a call to `m.gw.Analytics.RecordHit` for each request. This function sends data to a buffered channel which, if full, will block the request-handling goroutine. This can lead to performance degradation and resource exhaustion under heavy load or a DoS attack, as the analytics subsystem applies backpressure to the critical request path. This also increases the load on the underlying analytics storage, typically Redis.
Raw output
To ensure gateway stability, the call to send analytics data should be non-blocking. Modify the underlying `RecordHit` function to use a `select` statement with a `default` case. This will drop the analytic record if the channel is full, preventing the analytics pipeline from blocking request handlers during high-traffic scenarios.

// Record the hit
err := m.gw.Analytics.RecordHit(&record)
if err != nil {
mainLog.Errorf("Failed to record 404 analytics for stream on path '%s %s', %v", r.Method, r.URL.Path, err)
}
}
}
}

w.WriteHeader(http.StatusNotFound)
Expand Down
110 changes: 110 additions & 0 deletions gateway/proxy_muxer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/stretchr/testify/assert"

"github.com/TykTechnologies/tyk-pump/analytics"
"github.com/TykTechnologies/tyk/config"
"github.com/TykTechnologies/tyk/test"
)
Expand Down Expand Up @@ -365,6 +366,115 @@ func TestHandle404(t *testing.T) {
}...)
}

func TestHandle404WithAnalytics(t *testing.T) {
t.Run("track_404_logs disabled - no analytics record", func(t *testing.T) {
ts := StartTest(func(c *config.Config) {
c.Track404Logs = false
c.EnableAnalytics = true
})
defer ts.Close()

redisAnalyticsKeyName := analyticsKeyName + ts.Gw.Analytics.analyticsSerializer.GetSuffix()

ts.Gw.BuildAndLoadAPI(func(spec *APISpec) {
spec.Proxy.ListenPath = "/api/"
spec.UseKeylessAccess = true
})

// Cleanup before test
ts.Gw.Analytics.Store.GetAndDeleteSet(redisAnalyticsKeyName)

// Make request to non-existent path
_, _ = ts.Run(t, test.TestCase{
Path: "/nonexistent",
Code: http.StatusNotFound,
})

// Flush analytics to Redis
ts.Gw.Analytics.Flush()

// Verify NO analytics record was created
results := ts.Gw.Analytics.Store.GetAndDeleteSet(redisAnalyticsKeyName)
assert.Equal(t, 0, len(results), "No analytics record should be created when track_404_logs is disabled")
})

t.Run("track_404_logs enabled + analytics enabled - record created", func(t *testing.T) {
ts := StartTest(func(c *config.Config) {
c.Track404Logs = true
c.EnableAnalytics = true
})
defer ts.Close()

redisAnalyticsKeyName := analyticsKeyName + ts.Gw.Analytics.analyticsSerializer.GetSuffix()

ts.Gw.BuildAndLoadAPI(func(spec *APISpec) {
spec.Proxy.ListenPath = "/api/"
spec.UseKeylessAccess = true
})

// Cleanup before test
ts.Gw.Analytics.Store.GetAndDeleteSet(redisAnalyticsKeyName)

// Make request to non-existent path
_, _ = ts.Run(t, test.TestCase{
Path: "/nonexistent",
Code: http.StatusNotFound,
})

// Flush analytics to Redis
ts.Gw.Analytics.Flush()

// Verify analytics record was created
results := ts.Gw.Analytics.Store.GetAndDeleteSet(redisAnalyticsKeyName)
assert.True(t, len(results) > 0, "Analytics record should be created when track_404_logs and enable_analytics are true")

// Verify the record has correct fields
if len(results) > 0 {
var record analytics.AnalyticsRecord
err := ts.Gw.Analytics.analyticsSerializer.Decode([]byte(results[0].(string)), &record)
if err != nil {
t.Fatal("Error decoding analytics:", err)
}
assert.Equal(t, http.StatusNotFound, record.ResponseCode, "Response code should be 404")
assert.Contains(t, record.Tags, "404", "Tags should contain '404'")
assert.Contains(t, record.Tags, "path-not-found", "Tags should contain 'path-not-found'")
assert.Equal(t, "/nonexistent", record.Path, "Path should be '/nonexistent'")
}
})

t.Run("IP in ignored_ips - no record", func(t *testing.T) {
ts := StartTest(func(c *config.Config) {
c.Track404Logs = true
c.EnableAnalytics = true
c.AnalyticsConfig.IgnoredIPs = []string{"127.0.0.1"}
})
defer ts.Close()

redisAnalyticsKeyName := analyticsKeyName + ts.Gw.Analytics.analyticsSerializer.GetSuffix()

ts.Gw.BuildAndLoadAPI(func(spec *APISpec) {
spec.Proxy.ListenPath = "/api/"
spec.UseKeylessAccess = true
})

// Cleanup before test
ts.Gw.Analytics.Store.GetAndDeleteSet(redisAnalyticsKeyName)

// Make request to non-existent path from localhost (127.0.0.1)
_, _ = ts.Run(t, test.TestCase{
Path: "/nonexistent",
Code: http.StatusNotFound,
})

// Flush analytics to Redis
ts.Gw.Analytics.Flush()

// Verify NO analytics record (IP is ignored)
results := ts.Gw.Analytics.Store.GetAndDeleteSet(redisAnalyticsKeyName)
assert.Equal(t, 0, len(results), "No analytics record should be created when IP is in ignored_ips list")
})
}

func TestHandleSubroutes(t *testing.T) {
ts := StartTest(func(globalConf *config.Config) {
globalConf.HttpServerOptions.EnableStrictRoutes = true
Expand Down
Loading