Commit Graph
5 Commits
Author SHA1 Message Date
lukaszraczylo 87ebee9003 fix(auth): throttle repeated auth failures, rate-limit failure log
Fixes finding 3 from the 2026-08-22 security audit: authStore.identify
had no counter, no backoff, and no lockout on failed attempts, and
logger.go wrote one unbuffered stderr line per attempt including the
caller's RemoteAddr. An unauthenticated attacker got unlimited digest
guesses at line rate and could drive synchronous stderr writes on the
shared Traefik ingress.

- Add per-source-IP failure throttling to identify, reusing the existing
  in-process memoryStore (limits.go) as authStore's own dedicated
  instance rather than a new dependency: authFailureLimit (20) failed
  attempts per authFailureWindow (1 minute) throttles further attempts
  from that IP outright, including a subsequently valid key, and resets
  once the window elapses.
- clientIP reads r.RemoteAddr only, never a client-supplied header,
  matching the same trust boundary the rest of this package already
  assumes (routes_passthrough.go strips X-Forwarded-*/X-Auth-* before any
  outbound request).
- Rate-limit logAuthEvent's failure line via authStore.shouldLogAuthFailure,
  the identical technique limiter.logStoreError already applies to store
  errors. Successful-auth logging stays unbounded (bounded by legitimate,
  already-authenticated traffic elsewhere).
2026-08-22 15:26:25 +01:00
lukaszraczylo 90bb837a5d fix: escape request path in auth event logs
logAuthEvent (logger.go) formatted r.URL.Path with %s in both the
failure and success log lines. An unauthenticated client controls that
path and can embed a raw newline (%0A) in it, letting a single request
forge additional, attacker-chosen log lines. Switched both to %q, which
escapes control characters (newlines included) while still rendering
the path as a quoted string.

No test changes needed: both existing tests (llmgateway_test.go) assert
via strings.Contains on the bare path substring, which %q's quoted
output still contains.

Gates: go test ./... -count=1 green, gofmt -l . empty, go vet clean.
2026-08-20 15:06:34 +01:00
lukaszraczylo 4b849fd6c7 fix: force include_usage, auth event logging, credential-strip coverage
C1 (quota bypass): a client sending stream_options.include_usage=false
suppressed upstream usage reporting and escaped every token/cost budget,
since the non-streaming T12 estimate never covers a streaming response.
chatCompletion (provider_openai.go) now always forces include_usage=true
on the outgoing request, merged into a copy of the client's own
stream_options map so the caller's map is never mutated in place.
clientAskedUsage is derived from the client's own value via isTruthy, so
the usage-only chunk is still suppressed for a client that never asked
for it.

I1 (vacuous security tests): the X-Api-Key credential-strip set had zero
effective coverage — existing tests either never sent x-api-key, or
targeted anthropic, whose injectAuth overwrites x-api-key regardless of
whether the strip ran. Added one openai-passthrough and one MCP-proxy
test that authenticate via x-api-key itself and assert it arrives empty
upstream. Mutation-checked: both fail when "X-Api-Key": true is removed
from gatewayCredentialHeaders (verified, then restored).

I3 (auth-event logging): added Gateway.logAuthEvent (logger.go), called
at all six authStore.identify call sites (llmgateway.go's ServeHTTP,
routes_unified.go's handleModels). A failed attempt logs method, path,
and remote address; a success logs the resolved user's name and route.
Neither line ever includes the presented API key.

M2 (usage.estimated): runUnified now reads result.estimated and logs the
estimate explicitly when a non-streaming response's usage falls back to
the body-size heuristic, making the field genuinely read — removes the
stale //nolint:unused on limits.go's usage struct.

M8 (gosec consistency): secrets.go's file: secret path read now carries
the same "#nosec G304 -- operator-supplied file path from middleware
config" annotation as users_file.go's equivalent read.

Doc nit: mcp_a2a.go's validateTargetURLs comment now also names the
dot-only ("."/"..") rule validateConfigName enforces, not just character
set and reserved names.

Gates: go test ./... -count=1 (-race) green, gofmt -l . empty, go vet
clean, make yaegi-check OK, golangci-lint (gosec+staticcheck+unused+
errcheck+govet, global config) 0 issues.
2026-08-20 14:59:21 +01:00
lukaszraczylo d5bff28c5c fix: harden scaffold after review — manifest drift check, vet-safe logging, panic-path header guard 2026-08-20 00:26:28 +01:00
lukaszraczylo 2469918d7d feat: scaffold traefik-llmgateway plugin skeleton with manifest 2026-08-20 00:17:16 +01:00