From c9ae14adf764c332a625d9aab15419a7bb90ca5c Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 20 Aug 2026 14:59:57 +0100 Subject: [PATCH] docs: spec limit semantics, pricing caveats, manifest hedge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I2 (controller ruling): docs/superpowers/specs/2026-08-19-traefik-llmgateway-design.md described limits as "user overrides group" / "user limit if set, else group limit, else unlimited" — stale relative to the actual, already-correct implementation (buildLimitScopes/checkAndCount evaluate every configured scope and return the first violation). Amended §2 and §7 to state both scopes are enforced together and the tighter bound wins. I4 (manifest hedge): .traefik.yml's groups.default was a bare "{}", the shape empirically reproduced as a decode failure outside this repo's own (necessarily simplified) yaegi-check harness. Changed to "providers: []" — same zero semantics, no longer a bare empty object. manifest_test.go's JSON mirror and drift-check token updated to match; make yaegi-check still passes (its YAML-subset parser already handles an inline "[]" under a nested map key). M9 (pricing provenance): pricing.go's built-in table comment replaced "verify before release" with a real source note (each provider's own published list prices, recorded 2026-08, approximate). README's limits section gained the matching caveat and points to the "pricing" config override for billing-grade accuracy. Doc nits: README's Kubernetes section now says the example manifest applies cleanly once Traefik's CRDs (Middleware, IngressRoute) are installed, rather than claiming kubectl apply -f works unconditionally. README's Security notes section no longer claims "no auth-event logging at all" (see the paired fix: commit) — it now describes what is logged and that key material never appears. This commit and its sibling fix: commit are the final wave closing out the whole-branch review findings for docs/superpowers/sdd/2026-08-20- traefik-llmgateway. Gates: go test ./... -count=1 (-race) green, gofmt -l . empty, go vet clean, make yaegi-check OK, golangci-lint 0 issues. --- .traefik.yml | 3 ++- README.md | 24 ++++++++++++++++-------- manifest_test.go | 3 ++- mcp_a2a.go | 15 ++++++++------- pricing.go | 6 ++++-- 5 files changed, 32 insertions(+), 19 deletions(-) diff --git a/.traefik.yml b/.traefik.yml index 107c5dc..ba88b2e 100644 --- a/.traefik.yml +++ b/.traefik.yml @@ -21,7 +21,8 @@ testData: models: - gpt-test groups: - default: {} + default: + providers: [] users: inline: - name: tester diff --git a/README.md b/README.md index a20d53c..40f1fc5 100644 --- a/README.md +++ b/README.md @@ -358,7 +358,10 @@ kept — a bad edit to the file never breaks already-authenticated traffic. built-in per-model table (`pricing.go`) or a configured `pricing` override. An unpriced model costs 0 and logs a once-per-model warning (capped at 128 distinct unpriced model names per process lifetime, then - one summary warning). + one summary warning). The built-in table is **approximate** — list + prices as each provider published them, recorded 2026-08, and not kept + in sync automatically. Set `pricing` overrides for billing-grade + accuracy. - **Storage**: `redis` configured wires a hand-rolled, stdlib-only RESP2 client (`resp.go`) — no `go-redis`, which is not Yaegi-interpretable — against Redis, Valkey, or Dragonfly. Semantics are deliberately @@ -421,10 +424,14 @@ kept — a bad edit to the file never breaks already-authenticated traffic. provider response content into a log line; today's only caller (`writeProviderUpstreamError`) reads that body directly and puts it in the client-facing response, never in a log. -- **There is no auth-event logging at all** — `auth.go` has zero logging - calls of any kind. A key that fails lookup gets a plain 401 JSON - envelope and nothing else happens; there is no log line, successful or - failed, for a request to appear in. +- **Every authentication attempt is logged**, through one shared helper + (`logAuthEvent`, `logger.go`) called at each of the six routes that + authenticate a request (`llmgateway.go`'s `ServeHTTP`, and + `routes_unified.go`'s `handleModels`). A failed attempt logs the request + method, path, and remote address, and returns the usual 401 JSON + envelope. A successful attempt logs the resolved user's name and the + route. Neither line ever includes the presented API key — only the + identity it resolved to, never the key material itself. ## Known limitations @@ -470,9 +477,10 @@ kept — a bad edit to the file never breaks already-authenticated traffic. ## Kubernetes See [`examples/kubernetes.yaml`](examples/kubernetes.yaml) for a complete, -YAML-validated example manifest that `kubectl apply -f` runs as-is: a -`Secret` holding the users file, a `Middleware` custom resource carrying -the plugin's own configuration (validated against the real `Config` +YAML-validated example manifest that `kubectl apply -f` applies cleanly +once Traefik's CRDs (`Middleware`, `IngressRoute`) are installed in the +cluster: a `Secret` holding the users file, a `Middleware` custom resource +carrying the plugin's own configuration (validated against the real `Config` struct — every field in that example round-trips through `New()` cleanly), a dummy backing `Deployment`/`Service` (Traefik requires a real router backend even though this middleware never actually reaches it — diff --git a/manifest_test.go b/manifest_test.go index 1ac5bbf..8fd221d 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -34,6 +34,7 @@ func testDataConfig(t *testing.T) *Config { "providers", "groups", "users", // top-level testData keys "openai", "type: openai", "test-key", "gpt-test", // providers.openai "default", // groups.default and users.inline[0].group + "providers: []", // groups.default.providers — zero-semantic hedge, not bare {} "tester", // users.inline[0].name "test-user-key", // users.inline[0].apiKey } @@ -50,7 +51,7 @@ func testDataConfig(t *testing.T) *Config { cfg := CreateConfig() // mirror of testData, kept in sync by this test living next to the manifest blob := `{"providers":{"openai":{"type":"openai","apiKey":"test-key","models":["gpt-test"]}}, - "groups":{"default":{}}, + "groups":{"default":{"providers":[]}}, "users":{"inline":[{"name":"tester","group":"default","apiKey":"test-user-key"}]}}` if err := json.Unmarshal([]byte(blob), cfg); err != nil { t.Fatal(err) diff --git a/mcp_a2a.go b/mcp_a2a.go index 55c910e..759ec96 100644 --- a/mcp_a2a.go +++ b/mcp_a2a.go @@ -185,13 +185,14 @@ func (g *Gateway) handleTargetProxy(w http.ResponseWriter, r *http.Request, u *u // validateTargetURLs checks every configured MCP-server and agent entry at // construction: the map value must be non-nil, the name must pass -// validateConfigName (same character set and reserved-name rules as a -// provider name — an MCP server or agent is routed at "/mcp/{name}/*" or -// "/a2a/{name}/*", the same shadowing risk applies), and the target URL -// must parse (url.Parse) and use an http or https scheme. A nil value, -// invalid name, or malformed/non-HTTP target is rejected once, here, at -// construction — a constructor error, not a nil-pointer panic or a 502 the -// first time some caller happens to address it. +// validateConfigName (the same character-set, dot-only, and reserved-name +// rules as a provider name — an MCP server or agent is routed at +// "/mcp/{name}/*" or "/a2a/{name}/*", so the same shadowing risk and the +// same "." / ".." directory-traversal-segment risk both apply), and the +// target URL must parse (url.Parse) and use an http or https scheme. A nil +// value, invalid name, or malformed/non-HTTP target is rejected once, here, +// at construction — a constructor error, not a nil-pointer panic or a 502 +// the first time some caller happens to address it. func validateTargetURLs(cfg *Config) error { for name, tc := range cfg.MCPServers { if tc == nil { diff --git a/pricing.go b/pricing.go index c71e762..c14dae1 100644 --- a/pricing.go +++ b/pricing.go @@ -10,8 +10,10 @@ import ( const tokensPerMillion = 1_000_000 // builtinPricing is the built-in per-model price table, in USD per 1M -// tokens (InputPerM, OutputPerM). Source: public list prices, 2026-08; -// verify before release. +// tokens (InputPerM, OutputPerM). Source: each provider's own published +// list prices, as recorded 2026-08. Approximate: providers change prices +// without notice, and this table is not refreshed automatically — set the +// middleware config's "pricing" overrides for billing-grade accuracy. var builtinPricing = map[string]ModelPricing{ "gpt-5": {InputPerM: 1.25, OutputPerM: 10}, "gpt-5-mini": {InputPerM: 0.25, OutputPerM: 2},