fix(tollbooth): correctly read context value in ShouldSkipLimiter (bypass)

[desperatee]

Summary

ShouldSkipLimiter has two bugs in the context-value branch that combine into a complete bypass of any rate limiter configured with SetContextValue:

Bug 1 — r.Header.Get instead of r.Context().Value (line 160)

for contextKey, contextValues := range lmtContextValues {
    for _, contextValue := range contextValues {
        if r.Header.Get(contextKey) == contextValue {   // ← wrong source
            requestContextValuesDefinedInLimiter = true
            break
        }
    }
}

The adjacent first-phase check on line 142 and BuildKeys on line 247 both correctly use r.Context().Value(contextKey). This is the only site that reads from r.Header — almost certainly a copy-paste from a sibling function.

Bug 2 — fmt.Sprintf("%v", nil) != "" is always true (line 142)

reqContextValue := fmt.Sprintf("%v", r.Context().Value(contextKey))
if reqContextValue != "" {        // ← "<nil>" != "" is always true
    requestContextValuesDefinedInLimiter = true
    break
}

fmt.Sprintf("%v", nil) returns "<nil>" (5 chars), not "". The guard intended to skip when the context key is absent fires for every request, including those genuinely missing the key.

Combined behavior

For a limiter configured with lmt.SetContextValue("access-level", []string{"basic"}):

Request state	Expected	Actual
context has access-level=basic	rate limit applies	bypass
context has access-level=premium	rate limit applies	bypass
context has no access-level	skip	bypass (via bug 2)

Effect: any SetContextValue limiter is a no-op rate limiter.

Fix

• Line 160 → fmt.Sprintf("%v", r.Context().Value(contextKey)) == contextValue
• Line 142 → r.Context().Value(contextKey) != nil

Severity

CRITICAL — silent rate-limit bypass with no log signal. Any deployment using SetContextValue for per-tier limits (typical SaaS pattern) is unprotected.

Proof of Concept

ShouldSkipLimiter is responsible for filtering which requests should bypass the limiter based on context values configured via SetContextValue. Two bugs combine to make the filter unconditionally bypass the limiter for any non-empty contextValues list:

1. Line 160 reads r.Header.Get(contextKey) instead of the context value. The adjacent first-phase check at line 142 and BuildKeys at line 247 correctly call r.Context().Value(contextKey) — only this one site reads from the wrong source.
2. Line 142 uses fmt.Sprintf("%v", nil) != "", which evaluates to "<nil>" != "" (always true), so the first-phase "absent-key skip" never fires for missing context keys.

Net effect: any limiter configured with SetContextValue("key", []string{"some-value"}) skips rate limiting for all requests — including authenticated, anonymous, abusive, and trusted alike.

Steps to Reproduce

package tollbooth

import (
	"context"
	"net/http"
	"testing"

	"github.com/didip/tollbooth/v8/limiter"
)

func TestContextValueBypass(t *testing.T) {
	lmt := NewLimiter(1, nil).
		SetIPLookup(limiter.IPLookup{Name: "RemoteAddr"}).
		SetContextValue("access-level", []string{"basic"})

	req, _ := http.NewRequest("GET", "/", nil)
	req.RemoteAddr = "1.2.3.4:1234"
	//nolint:staticcheck
	req = req.WithContext(context.WithValue(req.Context(), "access-level", "basic"))

	if ShouldSkipLimiter(lmt, req) {
		t.Fatal("limiter bypassed despite matching context value")
	}
}

func TestContextKeyAbsenceShouldSkip(t *testing.T) {
	lmt := NewLimiter(1, nil).
		SetIPLookup(limiter.IPLookup{Name: "RemoteAddr"}).
		SetContextValue("tenant", []string{"premium"})

	// Request has NO 'tenant' context key
	req, _ := http.NewRequest("GET", "/", nil)
	req.RemoteAddr = "1.2.3.4:1234"

	if !ShouldSkipLimiter(lmt, req) {
		t.Fatal("limiter should skip when context key is absent")
	}
}

Both tests fail on master and pass after this fix.