mirror of
https://github.com/lukaszraczylo/traefik-llmgateway.git
synced 2026-10-12 04:54:45 +00:00
935df1a3f06292a3bf88a4f25eca7055a122083e
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 lukaszraczylo
|
935df1a3f0 |
docs(metrics): add README section, testData, and a scrape example
The metrics feature shipped in code only: no README section, no
metrics block in .traefik.yml testData, nothing under examples/. The
Go doc comments were the entire manual for a human operator or a
config-generating assistant.
Add a README Metrics section covering the served path, both auth
modes (admin bearer token, optional CIDR allowlist), the CIDR-sizing
warning with the live-reproduced SNAT failure mode, the modelLabel
cardinality trade-off, the exposed metric families, the cross-replica
aggregation contract, label escaping, and store-outage behaviour. Add
a MetricsConfig reference table matching the existing AdminConfig
one, and a metrics row in the top-level Config table.
Add metrics: {enabled: true} to .traefik.yml's testData, the
catalog-facing smoke-test config, deliberately without allowedCIDRs
so a copy-pasted example never skips the README's own sizing
guidance.
Add a metrics block to the Kubernetes Middleware example and a
VMServiceScrape resource: the target cluster runs VictoriaMetrics, not
Prometheus, so the CRD differs even though the exposition format does
not. Verified the VMServiceScrape shape (apiVersion, selector,
endpoints, path) against the victoriametrics/operator documentation
rather than assuming it. Notes that /metrics is served by Traefik
itself, not by any backend service, since this middleware intercepts
the path before a backend is ever reached.
|
||
|
lukaszraczylo
|
244dd6185f |
test(yaegi-check): verify label escaping under the interpreter
Add a third configured user whose name contains a double quote, a backslash, and a newline, then assert GET /metrics renders its Prometheus-escaped form and never the raw name. auth.go validates a user name as non-empty only, so this is a real, legal configuration, not a contrived one. The compiled suite already proves escapeLabelValue correct (TestMetrics_LabelValuesWithSpecialChars_Escaped, TestEscapeLabelValue_InvalidUTF8_Injective), but this codebase's own history is that a compiled pass has repeatedly said nothing about the interpreted shape. json.Marshal produces the JSON-escaped literal for the harness's own hand-built config document, kept separate from the Prometheus escaping the probe actually verifies in the rendered body. |
||
|
lukaszraczylo
|
d352379375 |
fix(metrics): honest rendering, injective escaping, aggregation docs
Three fixes from adversarial verification. writeProviderMetrics rendered a storeDown providerCounters entry as a hard zero, the same fabricated-reset hazard the request/token/cost families already guarded against. Skip a storeDown provider or provider-model entry instead of emitting it. escapeLabelValue ranged over the string as runes, and Go substitutes U+FFFD for every invalid UTF-8 byte. Two distinct configured names that both contain invalid UTF-8 could therefore render as the identical escaped value, colliding two series into one duplicate series and failing the whole scrape target. Escaping is now byte-oriented, which is injective and also avoids the per-rune decode cost on a large value. Every store-backed family's HELP text now states its aggregation contract: with Redis configured, the value is the gateway-wide total and identical on every replica, so summing across replicas overcounts by the replica count and a cost alert fires early. Aggregate with max() instead. Without Redis, the same family becomes genuinely per-process and sum() becomes correct instead. The rejection counter and the provider-healthy gauge get their own notes: the former is always per-process regardless of Redis, the latter reflects each replica's own independently-running discovery breaker. Also replaces the AllowedCIDRs doc example (10.0.0.0/8) with a cluster pod CIDR (10.42.0.0/16) and states plainly that a whole RFC1918 range is never correct, since a SNAT-ing router or load balancer makes external traffic arrive from inside it — reproduced live against the target cluster. Adds tests pinning three properties mutation-proofed against this suite: every storeDown skip (deleting any of them now fails TestMetrics_StoreDown_SkipsRatherThanFabricatesZero), the CIDR-check running before auth.identify (TestMetrics_ AllowlistedRequest_NeverTouchesAuthFailureTracking), and escapeLabelValue's injectivity under invalid UTF-8 (TestEscapeLabelValue_InvalidUTF8_Injective). Also covers the new rejection-counter pruning end to end through a real file-sourced reload. |
||
|
lukaszraczylo
|
5700dd482d |
fix(limits): never fabricate zero counters, bound the rejection counter
Two correctness fixes from adversarial verification. providerUsage returned every counter at its zero value on a failed-closed store read, with no signal distinguishing that from a real zero. metrics.go rendered the zero as a hard value. A single Redis blip therefore read as a counter reset, and the recovery scrape counted the whole prior total as a fresh increase, painting a phantom multi-thousand-request spike from one transient error. Add storeDown to providerCounters and set it on the failed-closed path, mirroring scopeUsage's existing contract. limiter.rejections grew without bound: nothing ever removed a scope whose user was renamed or deleted from a hot-reloaded users file, so a long-running process kept exporting a series for every name ever seen. Bound the map with the same current/previous generation-rotation authFailureTracker already uses, and evict a user's entry when authStore.maybeReload performs a real reload. maybeReload now returns whether it actually reloaded, so ServeHTTP only runs the eviction walk on that rare event, never on every request. Also corrects a stale comment: the rejection counter's store-down branch fires on every request during a full outage, not rarely, as an earlier comment claimed. |
||
|
lukaszraczylo
|
d421f49ff1 |
test(yaegi-check): verify the metrics endpoint under the interpreter
Compiled tests have repeatedly passed on this codebase while the interpreted shape failed, so the metrics feature needs its own probe in this harness, not just go test coverage. Enable metrics in the harness config, with modelLabel on so the opt-in provider-model breakdown runs interpreted too. Add exerciseMetricsRoute, driving four real requests: unauthenticated from outside the allowed CIDR block returns 401, a non-admin key returns 403, the admin key returns 200 with the expected TYPE lines and live provider and model data in the body, and a keyless request from inside the allowed CIDR block returns 200. Run the new probe last, after the breaker probe settles, since GET /metrics triggers the same maybeRefresh call every route does and the breaker probe's own hit counting is timing-sensitive. |
||
|
lukaszraczylo
|
d58eef9aa7 |
feat(metrics): add Prometheus text-exposition endpoint
Serve a hand-written Prometheus text-exposition document at a configurable path, defaulting to /metrics. The plugin is stdlib-only under Yaegi, so this cannot use a client library. Add MetricsConfig: Enabled gates the route, Path overrides it, AllowedCIDRs lets an unauthenticated caller scrape from a trusted source address, and ModelLabel opts into a per-(provider,model) breakdown that defaults off to avoid a series-count explosion. Reuse the existing admin bearer-token auth path instead of adding a second credential check. Reuse the batched, chunked reads GET /admin/api/overview and GET /admin/api/usage already use, so a scrape costs no extra store round trip. Expose request, token, and cost counters per scope, provider attempt and failure counters, a provider-health gauge from the discovery circuit breaker, and a budget-consumed-ratio gauge per limited scope for Prometheus alerting. Add a rate-limit-rejection counter in limits.go: this is the one number with no existing accounting to read back, so checkAndCount now increments a small in-process, per-process-lifetime counter on every violation. Escape every label value for backslash, double quote, and newline. An unescaped quote in a label value, such as a discovered model id, makes the whole scrape unparseable, and Prometheus reports an unparseable scrape as the target being down rather than as one bad line. Cover the auth gate matrix, exposition structure (TYPE per family, counters ending _total, no duplicate series), label escaping round-trips, the model-label opt-in, and counters reflecting real seeded usage rather than zeros. |
||
|
lukaszraczylo
|
208084ac83 |
merge: serve the Anthropic Messages API inbound
# Conflicts: # tools/yaegi-check/main.go |
||
|
lukaszraczylo
|
941a72bf7e | merge: per-provider discovery circuit breaker and health surfacing | ||
|
lukaszraczylo
|
3e1f188426 |
fix(routes): preserve response bytes and close remaining test gaps
Replace the alias-echo rewrite on the Anthropic passthrough branch with a byte-preserving field replacement. The prior version decoded the response into a Go map and re-marshaled it. This silently corrupted a JSON integer beyond float64's exact range. It also rewrote 1.0 to 1, HTML-escaped angle brackets and ampersands, and re-sorted every key. A tool_use block's "input" is arbitrary model-generated JSON. A long numeric id inside tool arguments could be silently corrupted on a route documented as passthrough. rewriteModelField now walks the response with a JSON decoder and copies every field but "model" as raw bytes. Add a test with a 20-digit integer, a 1.0 float, and a literal "<" character inside tool_use input. It asserts the response is byte-identical to the source apart from the model field. Add a negative test for the anthropic-beta header allowlist. It asserts Authorization and Cookie never reach the upstream. It also asserts the upstream sees exactly one x-api-key value: the gateway's own key, never the client's. A single Get() call could miss a second, leaked value, since Set() and Add() interact in a way a naive check would not catch. The test checks the Values() length explicitly. Add a test case for the native passthrough route's prompt-cache billing. It fails if cache_creation_input_tokens or cache_read_input_tokens drop out of the billed prompt count. This closes a production billing change that previously carried no coverage at all. Decouple response delivery from usage extraction on the passthrough branch. A response body read failure, or a malformed usage field, no longer both discard the response and misreport the failure as an upstream connection error. A read failure still returns an error, now correctly classified as a translation failure. A malformed usage field no longer blocks the response at all: the client still gets it, and the existing zero-usage estimate takes over for accounting. Return the adapter's own usage value on a chatCompletion error in the translated path, instead of discarding it. This has no effect while streaming stays rejected up front. It prevents the same critical bug from resurfacing once streaming support lands. Normalize tool_result content to a plain string before applying the error prefix. The prior version only prefixed a Go string. Anthropic's array content form, which Claude Code sends, lost the is_error signal entirely. Absent content now becomes an empty string instead of the JSON null OpenAI rejects. Treat "stream": null the same as an absent key. The prior fix for a non-boolean stream value also rejected null. An SDK that serializes an unset optional field as null must still get an ordinary non-streaming response. Drop a yaegi-check assertion that could not fail. The fixture configures no cache, so the header it checked was never set regardless of what the code under test did. A mutation pass confirmed this: it deleted the line the assertion was meant to guard, and make yaegi-check still reported OK. Document that this route now intercepts POST /v1/messages unconditionally, even under passthroughUnknown, where it previously fell through to the next handler. Add the note to both the new README section and Known limitations. Correct the passthrough table's claim about response bytes to describe the actual guarantee. |
||
|
lukaszraczylo
|
d0b55a37e9 |
fix(webui): match the health detail row color to its badge, add tests
ProvidersView.vue's accordion detail row hardcoded text-destructive red for any non-closed health state, while the trigger row's Badge deliberately used grey (secondary) for half-open and red (destructive) only for open. Add healthDetailClass so the detail row's color always matches the badge's own severity, instead of overstating a half-open probe as an error. Add a describe block for formatUntil to format.spec.ts, covering the unset sentinel, undefined, a future timestamp, and a past one. It had no test coverage since being added in the previous round. |
||
|
lukaszraczylo
|
46a3577162 |
docs: fix the provider health section and update the 6h default
The Provider health section claimed a broken provider stops being re-probed "every single interval forever", while its own third bullet said the opposite: under stock config the interval remained the binding constraint and nothing was actually suppressed. Rewrite the opening paragraph and the bullet to state the real relationship plainly: the backoff window only ever reduces re-probe frequency once it exceeds discoveryInterval, and the shipped defaults are chosen so that holds. Update every remaining reference to the old 30m maxOpenDuration default to the new 6h value, in the BreakerConfig reference table and the Provider health section, and explain what a misconfigured relationship between discoveryInterval and maxOpenDuration does. |
||
|
lukaszraczylo
|
a18580e930 |
fix(registry): make the breaker actually suppress re-probing
Address the design finding and remaining defects from round 3 of adversarial review. Raise defaultBreakerMaxOpenDuration from 30m to 6h. Round 2 fixed the breaker so it never retries more than plain interval throttling, but left it cadence-neutral: with maxOpenDuration shorter than the 1h default interval, the interval always dominated and backoff never suppressed anything. Measured over 24h with stock config, admits equaled the interval-only baseline exactly. At 6h, a permanently broken provider now backs off 1h, 2h, 4h, 6h, 6h, roughly 5 attempts a day instead of 24. The 24h cadence test now asserts admits are strictly fewer than baseline, not merely no worse. Correct the emitted openUntil to account for the interval floor. Because the interval gate applies unconditionally, the real next probe time is max(openUntil, lastRefresh+interval), not the raw backoff value alone. On pure defaults a fresh trip set the raw value to +1m while the real next probe was at +1h, a 59 minute skew the dashboard rendered as a countdown that reached zero and then silently vanished. snapshot now emits the corrected value while open; closed and half-open are unaffected. Revert a canceled half-open probe to open instead of leaving it at half-open. A half-open breaker reports healthy by design, so a canceled probe previously left a still-broken provider reading healthy until the next attempt happened to land. Reverting costs nothing extra, since the interval gate already governs when that next attempt is admitted either way. Rewrite the discovery-suppression claims in doc comments: the earlier wording claimed re-probing stops "every single interval forever" under all conditions, which the interval-floor fix from round 2 already made false, and which contradicted this file's own surrounding explanation. Both now state plainly that suppression only occurs once backoff exceeds discoveryInterval. Add a sentence to recordHealthLocked's doc comment stating that context.DeadlineExceeded counts as a plain failure, matching recordProviderAttempt's SHOULD-1 ruling, and a named test proving it. Tests: two new snapshot tests pin the openUntil correction in both directions. The cadence test's baseline comparison is now strict. A new test proves the zero value truly disables the breaker rather than tripping on the first failure. A new test proves a canceled context never gets misclassified as a failure, completing the bidirectional check the existing success-direction test already had. The concurrent test now asserts backoff stays within its ceiling and health stays a valid state on every iteration, rather than only checking for panics and data races. |
||
|
lukaszraczylo
|
8a3f9e7d00 |
feat(webui): surface discovery health state in the Providers tab
Add healthState and openUntil to AdminProviderView, and a formatUntil helper alongside the existing formatAgo/formatTimestamp pair, rendering a future ISO timestamp as "in Ns". ProvidersView.vue shows a Badge next to a provider's other status indicators, destructive for open and secondary for half-open, using the existing shadcn-vue Badge component rather than a hand-rolled indicator. A closed provider, the healthy common case, renders no badge at all, matching the existing convention for the per-model rate badge. The expanded accordion content adds a matching detail row. This was the brief's stated motivation for the admin surfacing work: a broken provider looked identical to a working one in the dashboard. That gap was previously closed at the API layer only; this closes it in the UI clients actually see. Regenerates admin_assets_gen.go from the rebuilt webui via make admin-ui, per this repository's own convention of committing the generated asset alongside its source change. |
||
|
lukaszraczylo
|
e82dabb1dc |
docs: document the discovery circuit breaker
Add BreakerConfig to the full configuration reference, alongside RetryConfig and CacheConfig, and add a breaker row to the top-level Config table. Add a new Provider health section explaining the three states, the failure classification, the neutral treatment of a canceled context, and the scope limitation: the signal reflects the discovery endpoint only and must not be the sole reason to refuse routing to a provider. Also document the new healthState and openUntil fields GET /admin/api/overview returns per provider, in the same sentence that already lists discoveryEnabled, modelRates, and modelMeta. |
||
|
lukaszraczylo
|
92b1af0705 |
test(yaegi-check): exercise the discovery circuit breaker interpreted
Add a breaker probe to the yaegi-check gate, alongside the existing handler coverage rather than replacing it. Drives a provider whose /v1/models 401s, the xiaomi production case, through the full closed to open to half-open to closed cycle against the real interpreter. This closes a gap adversarial review found: registry.go added a new interpreted named int32 type with a String method and a new classification call in recordHealthLocked, and nothing exercised either under Yaegi. limits.go's matchesSentinel already documents that this class of divergence between compiled and interpreted behavior is invisible to go test and only an interpreted probe can catch it. The probe configures a short breaker window (2 failures, 3 to 6 second backoff) and a 1 millisecond discovery interval on a dedicated "brk" provider, so the interval floor never binds and the breaker's own timing is what the assertions observe. It proves the breaker opens after sustained failures, suppresses discovery traffic for the whole open window, and closes again once the provider recovers. |
||
|
lukaszraczylo
|
24167b16f6 |
fix(registry): correct breaker cadence, classification, and cancellation
Address five findings from adversarial review of the discovery circuit breaker. All are behavioral corrections to registry.go; none touch the request path. Interval floor (blocking). tryBeginRefresh let an open breaker's own backoff window replace the plain discoveryInterval gate instead of adding to it. Since defaultBreakerMaxOpenDuration (30m) is shorter than defaultDiscoveryInterval (1h), a permanently broken provider was probed roughly twice as often as before this feature existed. The interval gate now applies unconditionally, closed or open alike, so backoff can only ever lengthen the retry cadence, never shorten it. Explicit failure classification. recordHealthLocked reused isTransient's retry-shaped semantics wholesale, including its exclusion of errRequestBuildFailed. A provider whose baseURL is malformed produced that exact error on every discovery cycle and never opened its breaker, because the old code routed it to a successful outcome. Health classification is now explicit: any non-nil, non-canceled error counts as a failure. Neutral cancellation. A context.Canceled outcome no longer counts as success or failure. Treating it as success previously closed an already-open breaker and wiped its backoff whenever a half-open probe was interrupted at shutdown, reporting a still-broken provider healthy. Zero-value safety. A providerState assembled without going through newModelRegistry now behaves like plain interval-only gating instead of opening on the first failure with a zero backoff, matching the type's own documented claim. Backoff overflow ceiling. Repeated doubling with no ceiling on maxOpenDuration could wrap the backoff negative after roughly 28 probes, silently disabling it. validateBreakerConfig now caps maxOpenDuration at 24 hours. Also renames modelRegistry.providerUsable and providerState.usable to discoveryHealthy, and documents that the signal reflects the discovery endpoint only. It must never be the sole reason to refuse routing to a provider whose request path works fine. feat/failover should combine it with a separate request-path signal fed by recordProviderAttempt. Tests: a 24-hour simulated cadence test proves the fixed gate admits no more attempts than plain interval throttling did before this feature. A concurrent test exercises the breaker fields under -race for the first time. Two tests replace a vacuous predecessor and distinguish neutral cancellation from treating it as success. One test proves errRequestBuildFailed opens the breaker. One test proves the zero value never opens. A rewritten healthy-provider test asserts behavior instead of reading the internal openUntil field directly. |
||
|
lukaszraczylo
|
25e73cb585 |
fix(routes): address review findings on the Anthropic Messages route
Preserve usage when response translation fails after a successful, billed upstream call. The route previously discarded already-billed tokens on that path. A client could induce unlimited free usage against any budget via a truncated tool-call response. Reject a streaming request with 400 instead of 501. Check it before the rate limiter runs. A 501 made anthropic-sdk-python retry the request, since it retries any status of 500 or higher. Each retry also counted against the caller's rate limit before this fix. Preserve block order when translating an Anthropic message to OpenAI's shape. A tool_result block before other content must stay before it. OpenAI otherwise rejects the translated conversation. Echo the client's requested model alias in both the passthrough and the translated response paths. The passthrough branch previously returned the upstream's own model id instead. Forward the anthropic-beta header to an anthropic-type upstream through an explicit allowlist. Never forward Authorization, x-api-key, Cookie, or hop-by-hop headers. Let the adapter's own credential injection win regardless. Bill Anthropic's prompt-cache tokens. anthropicUsagePayload now reads cache_creation_input_tokens and cache_read_input_tokens too. It folds all three into the billed prompt count. This applies to this route, the existing outbound Anthropic path, and the native passthrough route. Extract the metered-request pipeline shared by the unified route and this route into runMeteredCall. Thread an envelope writer through it so each route answers in its own error shape. This removes the duplication the original brief warned against. It also lets the rate-limit and body-admission error paths answer in this route's own shape. Treat a non-boolean "stream" value as a streaming request. A string or number value previously bypassed the streaming check silently. Emit max_completion_tokens instead of max_tokens for OpenAI's reasoning models. Those models reject max_tokens with a 400. Preserve tool_result.is_error by prefixing string content with "Error: ". OpenAI's tool message carries no error field of its own. Log a warning when the thinking field is dropped instead of discarding it silently. Classify a response-translation failure distinctly in the log. An operator must never see a genuine translation bug reported as an upstream connection error. Document the route in README.md. Cover both auth headers, the non-streaming limitation, and which providers translate versus pass through. Add the reviewer's yaegi-check probes to tools/yaegi-check/main.go, adjusted for the fixes above. |
||
|
lukaszraczylo
|
619931b2b2 |
feat(admin): surface provider health state in overview
Add HealthState and OpenUntil to adminProviderView, GET /admin/api/overview's per-provider row. HealthState mirrors the discovery circuit breaker's state: closed, open, or half-open. OpenUntil is the zero time.Time unless the breaker is open. A provider with discovery disabled, or one that has never failed a refresh, always reads closed. This keeps the dashboard's default view unchanged for a healthy deployment. Both fields come from providerSnapshot, read in the same locked pass as the existing lastRefresh/lastErr fields, so a concurrent finishRefresh can never hand the dashboard a breaker state that disagrees with the rest of the row. |
||
|
lukaszraczylo
|
e9ce6b873d |
feat(registry): add per-provider discovery circuit breaker
Add a three-state circuit breaker to providerState: closed, open, and half-open. It is driven only by discovery-refresh outcomes, never by request-path traffic. A provider whose refresh fails breakerThreshold times in a row opens the breaker and backs off before its next half-open probe, instead of retrying every discovery interval forever. A failed probe reopens the breaker with a doubled backoff, capped at breakerOpenMax. A successful probe closes the breaker again, so a recovered provider never stays dead. The failure classification reuses limits.go's isTransient and isDeadlineExceeded, the same rule recordProviderAttempt already applies to request-path attempts. It does not add a second classification. Add Config.Breaker (BreakerConfig), validated and defaulted by validateBreakerConfig. Every default preserves current behavior for a provider that never fails: a closed breaker never changes the existing interval-based refresh gating. Export modelRegistry.providerUsable(name) as the queryable health predicate. A sibling branch (feat/failover) will use it to skip unusable providers before routing. This commit does not touch the request path or routes_unified.go. |
||
|
lukaszraczylo
|
d47dd06a46 |
feat(routes): serve inbound Anthropic Messages API
Add POST /v1/messages, serving Anthropic SDK clients directly instead of outbound-only support. A request resolved to an anthropic-type provider passes through untranslated. A request resolved to any other provider type translates to and from OpenAI's chat-completion shape through the resolved adapter's own chatCompletion, since that method's contract is OpenAI-shape in and out regardless of provider type. The route reuses admitRequest, readAndDecodeUnifiedBody, registry.resolve, withAttemptRecorder, and limiter.account exactly as routes_unified.go does, so groups, model resolution, limits, accounting, and caching semantics stay identical to the unified route. Client-facing errors use Anthropic's envelope shape. A streaming request is rejected explicitly with a 501, never silently answered non-streamed; streaming support is a follow-up branch. Auth reuses authStore.identify and logAuthEvent unchanged. presentedKey already reads both x-api-key and Authorization: Bearer, so no new credential-checking code was needed. Add openAIRequestFromAnthropic and anthropicResponseFromOpenAI to translate_anthropic.go, the mirror of the existing outbound translators, plus their supporting helpers and unit tests. |
||
|
lukaszraczylo
|
9dc67dedbb |
feat: add anonymous usage reporting, matching traefikoidc
Release builds send one anonymous "plugin loaded" ping per process, so the project can see roughly how many installs exist and which versions are in use. The payload is project name, version and timestamp. Nothing else leaves the pod: no identifiers, no hostname, no configuration, no provider names, no keys, no request data. Three things keep it from firing when it should not. Builds from source carry the devPluginVersion sentinel and stay silent, so developer checkouts and every test run never phone home. The ping is sent only after the plugin constructs successfully, so a rejected configuration is never counted as an install. A sync.Once holds it to one per process, because Traefik calls New once per route and oss-telemetry does not deduplicate on the client side. Operators of a release build opt out with DO_NOT_TRACK=1, OSS_TELEMETRY_DISABLED=1, or TRAEFIK_LLMGATEWAY_DISABLE_TELEMETRY=1. README.md documents the full payload and the opt-out; a plugin that phones home without saying so in its own README would be indefensible. oss-telemetry is the plugin's first non-stdlib RUNTIME dependency, which has consequences for the Yaegi gate: - vendor/ is no longer excluded from yaegi-check's GOPATH copy. It was excluded on the reasoning that it held only test-only testify and _test.go files are never interpreted. That reasoning no longer holds: leaving vendor out would make the interpreted copy fail to resolve an import the real plugin tree resolves fine, so the gate would stop representing production. - yaegi-check now stamps a fake release version into its GOPATH copy so New's telemetry gate is PASSED rather than short-circuited, and the vendored package is actually called under the interpreter. Import resolution alone is not coverage; a cross-package call into interpreted third-party code is its own risk. Telemetry is suppressed by env vars set before anything is interpreted, so the gate never emits a real ping. - A new assertion reads the version back through GET /admin/api/overview. Without it a silently failed stamp would leave the dev sentinel in place and the harness would still print OK while covering nothing. Verified by disabling the stamp and watching the gate fail. shouldSendTelemetry is split out from sendStartupTelemetry so the decision is table-testable without a network, since oss-telemetry's endpoint is deliberately not overridable from consuming code. Also brings the version-gated documentation up to date, since the same README edit touches it. The latest release is v1.0.3, so the pre-1.0 experimental warning has met its own stated condition and is replaced by a semver stability statement in README.md and dropped from .traefik.yml's catalog summary. The catalog now serves the released tags (verified: the v1.0.3 download endpoint returns 200), so the "not yet published" and "analyzer has not been run" claims are corrected, the catalog form of the static config is documented as usable, and examples/kubernetes.yaml pins v1.0.3 instead of v0.1.0. |
||
|
lukaszraczylo
|
0346c3ed39 |
fix(registry): log model-id collisions at WARN, not ERROR
A model id served by two configured providers is resolved deterministically by the registry, the winner is named in the message, and no request is affected. Logging it at ERROR put 144 ERROR lines in a healthy pod's boot log on a cluster with overlapping openai, openai-audio, copilot and macstudio catalogs, which is exactly the noise that hides a real failure. Adds Gateway.warnf beside logf and errorf, and a warn field on modelRegistry carrying it. The field is set by the production caller after newModelRegistry returns rather than added as a fourth constructor parameter: 45 call sites pass a logger today, almost all of them tests passing a no-op, and threading a parameter through all of them would bury a one-line severity change in unrelated churn. A nil warn falls back to log, so a registry built without one keeps its current behaviour instead of panicking or dropping the line. registry.warnf takes an already-formatted string so both call shapes stay single-variadic-argument. Yaegi v0.16.1's CFG builder panics on a call to a struct FIELD of variadic func type with two or more variadic arguments, which is the restriction the log field beside it already documents. Verified under the real interpreter, not only compiled: yaegi-check drives a genuine two-provider collision and now prints WARN. The severity test fails if the call site reverts to log, so the level cannot drift back unnoticed. Only this message moves. Genuine failures — discovery errors, metadata capture panics, encoding failures — stay at ERROR. |
||
|
lukaszraczylo
|
5a34c5a52c | merge: federated MCP auth and fan-out hardening (audit round 2) | ||
|
lukaszraczylo
|
de42192426 |
refactor(mcp): restore errors.Is for the oversize-response sentinel
Federation's tools/call oversize branch matched a string marker with strings.Contains instead of wrapping a sentinel. The stated reason was that no gate exercised this path under Yaegi, so errors.Is was unproven here. The caution was misplaced. errors.New returns a compiled *errors.errorString and fmt.Errorf a compiled *fmt.wrapError, so declaring the sentinel variable in interpreted code leaves the entire chain errors.Is walks compiled. Neither documented trap applies: the matchesSentinel trip-wire covers a plugin-declared error TYPE, and the comma-ok trap needs a plugin-declared interface. retry.go has matched errRequestBuildFailed this way through a multi-%w wrap since v0.2.0. The string check was also strictly worse: err.Error() is itself an interpreted-to-compiled dispatch, so it verified nothing extra, and matching on text that partly comes from the backend admits false positives a sentinel cannot have. Rather than settle this by argument, tools/yaegi-check now drives POST /mcp tools/call against a backend whose response exceeds the cap, and asserts the interpreted plugin reports "response too large". Breaking the match makes the gate fail with "upstream error", so the probe has teeth. This closes the objection permanently instead of answering it in a comment. Also in doBackendJSONRPC, check the HTTP status before the size cap. A backend failing with a large HTML error page now reports "upstream returned HTTP 500", which is actionable, rather than sending the operator after a payload-size problem that does not exist. Two comment corrections carried forward from review: the fan-out semaphore is not the same shape as limiter.spawnTokens (spawnTokens drops work via a default case, this one blocks and must never skip a server), and the shared fan-out deadline gives later batches the remaining budget, not a fresh one, so uniformly slow backends yield a partial list. |
||
|
lukaszraczylo
|
452be5151c |
fix(mcp): stop weighting tools/list budget, widen fan-out, split caps
Addresses critical finding 3 and important findings 3-4 from security review round 2, 2026-08-22. Critical finding 3 — the tools/list-per-backend weight charged against the caller's own requests-per-minute/day budget (security audit finding 1b) was not default-preserving: both live groups have an empty MCP allow-list (all 11 servers), so it silently cut every configured requests-per-minute budget by ~11x with no config change, and examples/kubernetes.yaml's shipped requests-per-minute: 10 would 429 the very first tools/list call ever made. Reverted: checkAndCountWeighted removed from limits.go, handleMCPFederated back to a plain checkAndCount. The real amplification stays fully visible to an operator via countTargetRequests' existing per-server 'mcp' target-scope counters — a separate scope from the user/group budget — rather than silently consuming tenant quota. The fan-out semaphore (below) is the actual protection against the amplification finding 1 described. Important finding 3 — the fan-out semaphore (4 concurrent backends) tripled worst-case latency (907ms measured at 300ms/server, was ~300ms) and pushed the hung-backend worst case to ceil(11/4)x20s=60s, past the ~30s timeout many MCP clients use. Raised to 8, and every backend in one tools/list call now shares ONE overall toolsListBackendTimeout deadline (fanoutCtx) instead of each getting a fresh one from its own staggered start — restoring this function's original 'bounds the whole call regardless of server count' invariant even with bounded concurrency. Important finding 4 — the 4MiB response cap (sized for the tools/list fan-out's memory-amplification risk) silently applied to tools/call too, where exactly one backend is ever contacted and a legitimate result (an image, an extracted document) can be much larger. tools/call now uses mcpBackendCallResponseMaxBytes (10MiB, the pre-finding-1c budget); tools/list keeps the 4MiB cap. A response that hits either cap is detected via a cap+1 read (never silently truncated into a confusing parse failure) and reported to the client as 'response too large'. |
||
|
lukaszraczylo
|
3536c32667 |
fix(auth): let a valid key always win over IP throttle
Fixes critical finding 1 from security review round 2, 2026-08-22: identify checked the per-source-IP throttle BEFORE verifying the presented key. This plugin runs behind a Kubernetes Service, and with externalTrafficPolicy: Cluster (this operator's own deployment), r.RemoteAddr is the node's own SNAT address shared by every external caller reaching that node — not a per-tenant address. 20 failed requests from anywhere on the internet sharing that node locked out all 10 live users, including the admin dashboard's own poll. - identify now verifies the key FIRST, unconditionally: a valid key always succeeds before any throttle state is read. Every failure path still records against the caller's IP, but throttling is now pure observability (bounding log volume and tracker memory), never enforcement — an attacker with no valid key already got 'false' either way, so nothing is lost against brute force. - Replaces the per-source-IP tracker's backing store: the general- purpose memoryStore (limits.go) has no size cap and a full mutex-held sweep that scales with tracked-IP count (~83ms at 1M IPs). authFailureTracker is a bounded, two-generation, O(1)-rotation structure capped at 2x50,000 entries (important finding 1). - logAuthEvent now emits a distinct, separately rate-limited 'auth throttle:' line the first time a source crosses the limit (important finding 5), and both the routine and throttle-engaged lines report how many events they suppressed since the last line actually written, so several distinct attacker IPs in one window no longer collapse into a single indistinguishable line (important finding 6). - The success path no longer touches the throttle tracker's mutex at all (important finding 2). |
||
|
lukaszraczylo
|
d2a0c4860f |
fix(auth): reject duplicate and empty user names
Fixes finding 5 from the 2026-08-22 security audit: newAuthStore and
replaceFileUsers rejected duplicate API keys but not duplicate names, and
counters are keyed on u.name (routes_unified.go), so two users sharing a
name silently shared llmgw:user:{name}:* counters and both appeared in
buildAdminUsage; an empty name collapsed every such user into
llmgw:user::*.
- buildEntry now rejects an empty user name outright — the source of
both bugs, since duplicate names and an empty name are the same root
cause (two config entries resolving to the same counter key).
- newAuthStore and replaceFileUsers each reject a duplicate name WITHIN
their own source (two inline users, or two file users) — but not
across sources: a file user overriding an inline user of the same name
stays intentional and unaffected, per replaceFileUsers' own documented
merge semantics.
- Deliberately does NOT extend configNamePattern's character
restriction to user names: that pattern exists because provider/
mcpServers/agents names are embedded as URL path segments, which a
user name never is here. Restricting the charset further risked
rejecting real operator-chosen names this round has no visibility
into; the live users file's steve-cw/kevinsandom already pass the
non-empty check unaffected.
Also adds TestDefaultPreserving_LiveClusterShape_LoadsAndServesIdentically
(admin_test.go), modeling the live cluster's 5 friends + 4 home users and
11 MCP servers to prove all five fixes in this round load and serve
identically at that scale.
|
||
|
lukaszraczylo
|
800f63ac53 |
fix(admin): chunk usage batch reads to bound connection contention
Fixes finding 4 from the 2026-08-22 security audit: buildAdminUsage sent every user's, group's, and the total scope's counters (8 keys each) to limiter.currentUsage in ONE storeGetMulti round trip. With 1,000 users that is ~8,000 GETs pipelined through the single mutex-guarded store connection, during which every concurrent checkAndCount from live LLM traffic queues behind it (up to respCallTimeout) — and the admin dashboard polls this endpoint every 5s. Add chunkedCurrentUsage: splits the scopes slice into batches of at most adminUsageChunkScopes (200) before calling currentUsage, letting live traffic interleave between chunks while each chunk still batches its own scopes in one round trip. At or under 200 scopes (the live cluster's 9 users) this makes exactly one call, identical to today's behavior — the payload shape is unchanged, so no webui coordination is needed. |
||
|
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). |
||
|
lukaszraczylo
|
8bc54d15b5 |
fix(mcp): recover panics in tools/list fan-out goroutines
Fixes finding 2 from the 2026-08-22 security audit: the per-server goroutines in mcpFederatedToolsList carried only defer wg.Done(), unlike every other off-request goroutine in this package (limiter.spawn, modelRegistry.refreshProvider/captureModelMetadata), which all guard against a panic crashing the whole shared Traefik process. This codebase has repeatedly hit interpreter-level panics (registry.go reflect.Set, cache.go CFG index-out-of-range, providers.go errors.As), so an unrecovered panic here is not hypothetical. Add the same defer-recover pattern used at the other three sites: a panicking backend now degrades to "that server failed" and is logged, exactly like an HTTP error from it, instead of taking down the process. |
||
|
lukaszraczylo
|
26ff1a8b5b |
fix(mcp): bound federation tools/list fan-out and weight admission by backend count
Fixes finding 1 from the 2026-08-22 security audit: mcpFederatedToolsList spawned one unbounded goroutine per allowed MCP server, each able to issue up to 4 sequential HTTP calls reading up to 10MiB per response, while checkAndCount charged the caller only 1 request regardless of how many backends the fan-out actually contacted. With the 11 production servers, one client tools/list call could drive up to 44 outbound requests and hold ~110MiB concurrently, all for the cost of a single request against the caller's own rate limit. - Bound the fan-out to mcpFederatedFanoutConcurrency (4) concurrent backends via a buffered-channel semaphore, the same shape limiter.spawnTokens already uses. - Add limiter.checkAndCountWeighted so a tools/list call charges len(allowedMCPServerNames) requests against the caller's requests-per-minute/day budget instead of undercounting the fan-out as one; every other route keeps weight 1 via checkAndCount's own unchanged wrapper. - Shrink the per-backend response cap from maxRequestBytes (10MiB, sized for a client's chat/embeddings body) to mcpBackendResponseMaxBytes (4MiB, sized for a JSON-RPC reply), bounding worst-case concurrent memory to 4 x 4MiB = 16MiB. |
||
|
lukaszraczylo
|
8ab617767c |
fix(limits): cut body-admission bounds to [8,64] on measured heap cost
Round 3 sized these from the isolated json.Unmarshal peak (~51.5MB). Measured end-to-end through the real handler it is ~80-108MB per slot, so a 256 ceiling allows a 21GB peak under adversarial load — above any realistic pod budget, and the reference deployment limits Traefik to 512Mi. The slot is now held only across read+decode, so a smaller bound costs legitimate throughput almost nothing. |
||
|
lukaszraczylo
|
f405340d32 |
test(integration): exercise passthrough model/path restriction under Yaegi
SHOULD-7, round 3, 2026-08-22 review -- a report-correction gap, not a
code defect (the reviewer verified peekPassthroughModel/
scanTopLevelModel and allowsPassthroughPath directly via the repo's own
Yaegi harness). Every group/user this integration suite drove native
passthrough through ("eng", "limited", "hotreload") left models and
passthroughPaths unset, so hasModelRestriction() was always false and
allowsPassthroughPath's non-empty-list branch was never reached -- the
suite itself never exercised security findings 2/3's enforcement code
under a real Traefik+Yaegi process, contrary to an earlier claim.
Added a dedicated "restricted" group (models: ["gpt-mock"],
passthroughPaths: ["v1/chat/completions"]) and its own user (dave),
isolated from every other group/test in this file, plus
TestPassthroughModelAndPathRestriction: an allowed model on the allowed
path (200, proxied for real), a denied model on that same allowed path
(403 -- proves the model peek runs), and an ALLOWED model on a path
outside the allowlist (403 -- proves the path check alone, not the
model check, is what fires there). Verified passing via `make
integration` against the real docker-compose stack.
|
||
|
lukaszraczylo
|
03fe1d6589 |
docs(readme): document body-admission limit and the narrower unified cap
Round-3, 2026-08-22 review (SHOULD-6): maxInFlightBodyRequests, the new 503, and the unified routes' narrower 4MiB decode cap appeared nowhere in the README -- a gap on a public Catalog plugin. Also corrected the now-stale "images/generations and audio/speech share the unified routes' 10MiB JSON cap" line: the unified chat/embeddings routes moved to 4MiB while the media routes kept 10MiB, so they no longer share a cap at all. Added: - maxInFlightBodyRequests to the top-level Config reference table. - A new "Body-admission limit" subsection under Limits and accounting, covering the semaphore's scope (read+decode only, not the whole request), the 503+Retry-After behavior, the self-tuning default, the 4MiB unified cap, and the explicit note that native passthrough is not gated by this at all (streams rather than decodes). |
||
|
lukaszraczylo
|
64126db7d4 |
docs(passthrough): drop out-of-scope "." rejection, correct %25 comment
Two doc/scope corrections from the round-3, 2026-08-22 review (FOLDED +
SHOULD-5), no behavior change to the traversal bypasses finding 4 itself
fixed:
- hasTraversalSegment rejected an exact "." segment alongside "..",
outside finding 4's actual scope: "v1/x/./y" is a legal, non-traversal
path and was silently getting 400'd. Dropped the "." check; only ".."
is rejected again, matching every other traversal rule this function
enforces.
- The double-encoding rejection's own comment claimed it "rejects an
intentionally double-encoded %25" -- overstated. "%25" is the correct,
single, standards-conforming encoding of a literal "%" (RFC 3986);
after exactly one unescape pass, a legitimate single-encoded literal
"%" is byte-for-byte indistinguishable from a genuine double-encoding.
The behavior (fail closed) is still correct and unchanged -- refusing
an occasional literal "%" is a far smaller cost than admitting a real
double-encoded traversal -- but the comment now says so honestly,
including the concrete false-positive example ("v1/100%25done" decodes
to "v1/100%done" and is rejected even though it names a real,
non-malicious resource).
Tests: added "v1/x/./y" (must not 400) and "v1/100%25done" (the
documented false positive, still 403s) to hasTraversalSegment's table.
|
||
|
lukaszraczylo
|
c69210fd91 |
fix(routes): release body-admission slot after decode, not whole handler
Fix an availability regression the round-2 body-admission semaphore shipped (MUST-1, round 3, 2026-08-22 review): `defer release()` spanned the WHOLE handler in runUnified and all three media handlers, including the upstream round trip and response streaming. The stated goal was bounding decoded-body memory during the read+json.Unmarshal step; the shipped code instead bounded total in-flight requests, since an LLM completion is I/O-bound for seconds to minutes. Measured, zero-config, 400 concurrent chat requests: 220 succeeded, 180 got 503, on a cap a 2-vCPU pod would size at 16. Narrowed the held window to read+decode only, via new helpers that do the acquire/decode/release internally and return before the caller ever resolves a model or calls the adapter: - routes_unified.go: readAndDecodeUnifiedBody (runUnified no longer acquires/defers at all). - routes_media.go: decodeMediaJSONRequest now claims/releases its own slot (images.generations, audio.speech); a new readAdmittedCapped helper does the same around audio.transcriptions' readCapped call — narrower still, since that route never json.Unmarshal's into a map[string]any at all. Every release still goes through `defer` inside these narrow helpers, not a bare post-call statement, so a slot is released exactly once on every path including a panic unwinding through the read/decode itself. Since the held window shrank from "whole request" (seconds-minutes) to "one decode" (low milliseconds), raised the self-tuned default: bodyAdmissionCapPerCPU 8->24, floor 16->32, ceiling 128->256 — justified in llmgateway.go's doc comment against the measured adversarial-shape decode peak (~51.5MB LIVE heap, ~189MB TotalAlloc for a 4MiB adversarial body), not the old, no-longer-accurate ~170MB whole-pipeline estimate. Also clamped an explicit Config.MaxInFlightBodyRequests override to 10,000 (previously accepted verbatim, so e.g. a fat-fingered MaxInt would have silently disabled the bound). Corrected maxUnifiedRequestBytes' own DoS-math doc comment (SHOULD-3): the prior "~11x general case" framing was wrong — a realistic single inline base64 image measures ~1.0x (no amplification at all), while the adversarial many-small-values shape is what actually amplifies (12.3x LIVE / ~45x TotalAlloc, both now stated with real measured numbers instead of a single misleading multiplier). Documented two real, previously-silent behavior changes from finding 1a's admission-before-decode reordering (SHOULD-4): an over-budget caller whose request would otherwise have 400'd/404'd now gets 429 instead, and a malformed/unroutable request now consumes rate-limit budget it previously did not. Both are arguably improvements but were not visible from the diff alone. Replaced the sequential zero-config admission test, which could not actually exercise contention (MUST-2: the reviewer reran its exact shape with MaxInFlightBodyRequests=1 and it still passed), with a genuinely concurrent one: n = defaultBodyAdmissionCap()+20 real goroutines against a gated upstream, asserting (a) zero 503s across all n and (b) the semaphore holds zero slots while all n are parked in the upstream -- the regression's own regression test. |
||
|
lukaszraczylo
|
60036c43df |
fix(passthrough): strip dangerous upstream response headers
Close the response-header relay hole (security review finding 5, round 3, 2026-08-22): proxyUpstream copied every upstream response header to the client except hop-by-hop ones -- an upstream provider, or an in-cluster MCP/A2A target reached with no credential trust boundary of its own, could set Set-Cookie on the gateway's own origin (the same origin serving the WAN-exposed /admin dashboard), or return Access-Control-*/Strict-Transport-Security/Content-Security-Policy headers to rewrite the browser's security or CORS policy toward that origin. Added dangerousResponseHeaders (exact-match: Set-Cookie, Strict-Transport-Security, Content-Security-Policy) and dangerousResponseHeaderPrefixes (Access-Control-*), stripped in proxyUpstream right where hop-by-hop headers already are -- so both callers (handlePassthrough here, handleTargetProxy in mcp_a2a.go) get the fix for free. A deny-list, not an allowlist: grepping this repo finds it only ever reads Content-Type and Retry-After off a response (the latter only for the unified routes' own internal retry decision, never from passthrough). Everything else -- including provider rate-limit telemetry headers a client's own SDK commonly reads -- is opaque to this gateway but real passthrough transparency; an allowlist would silently break that for every header this package doesn't already know to name. This mirrors the request side's own already-hardened deny-list shape (dangerousClientHeaders/providerCredentialRetargetHeaders). Test: an upstream setting all four dangerous headers plus Content-Type, Retry-After, and an ordinary custom header proves the four are stripped while the rest reach the client unchanged. |
||
|
lukaszraczylo
|
ff819e01d7 |
fix(passthrough): catch path-parameter, backslash, and double-encoded traversal
Close three further traversal bypasses hasTraversalSegment's single-pass
decode-and-split missed (security review finding 4, round 3,
2026-08-22), each a real upstream convention:
- Path parameters ("..;/x", the Tomcat/Spring RFC 3986 Section 3.3
convention): a segment's identity is everything before its first ";".
Every segment now has its ";"-suffix stripped before the traversal
comparison.
- Backslash normalization ("..%5c..%5cx", decoding to "..\..\x"):
Windows/.NET and other backslash-normalizing upstreams treat "\" as an
equivalent separator. The decoded string now has every "\" replaced
with "/" before splitting.
- Double-encoding ("%252e%252e/x"): one unescape pass decodes this to
"%2e%2e/x" -- still containing "%". Rather than decode a second time
(which invites an unbounded decode loop), a rest that still contains
"%" after the one legitimate unescape pass is now rejected outright.
Also now rejects an exact "." segment, not just "..", closing the same
class of ambiguous-normalization bypass on a self-reference segment.
Literal ".." and single-encoded "..%2f" keep their existing, already
correct behavior; a legitimate encoded "/" within one segment (a2Fb) and
a genuine, non-traversal use of ";" on an unrelated segment are both
still forwarded.
Tests: hasTraversalSegment's table gains one case per new bypass plus
adjacent non-traversal cases proving the fix doesn't over-reject, and
the end-to-end ServeHTTP traversal test gains the same three variants
driven through the full request path.
|
||
|
lukaszraczylo
|
d70e5fc1df |
fix(auth): decode rest before matching the passthrough path allowlist
Close the %2f allowlist bypass (security review finding 3, round 3,
2026-08-22): allowsPassthroughPath and hasTraversalSegment (routes_
passthrough.go) reasoned about rest asymmetrically -- the traversal check
decodes it first, the allowlist matcher ran path.Match against the raw
escaped form. path.Match("v1/*", "v1/chat%2fcompletions") reports true
(one escaped segment, no literal "/" to split it), while the decoded
equivalent, "v1/chat/completions", is two segments and does not match
"v1/*". A group restricted to PassthroughPaths: ["v1/*"] therefore let
"/openai/v1/fine_tuning%2Fjobs" through -- a path an upstream that
itself decodes %2F would read as a different, unauthorized endpoint.
allowsPassthroughPath now url.PathUnescape's rest before matching -- the
same decode hasTraversalSegment already applies -- and denies a rest
that fails to unescape at all, mirroring hasTraversalSegment's own
fail-closed rule for malformed encoding. The empty-pattern fast path
(matchesGlob's own empty-means-all contract) is checked first, before
any decoding: a group with no PassthroughPaths configured -- every live
group today -- keeps allowing every request exactly as before, malformed
encoding included; hasTraversalSegment (unconditional, later in
handlePassthrough) remains the gate for that case.
Tests: a direct unit test proving the decoded three-segment form is
denied (both via the encoded and literal spellings), a single-segment
encoded value still matches, malformed encoding denies for a restricted
group and is unaffected for an unrestricted one, and an end-to-end
ServeHTTP test reproducing the exact finding scenario.
|
||
|
lukaszraczylo
|
a221b4d2eb |
fix(passthrough): fail closed on a model peek that never closes the object
Close the residual gap in the duplicate-"model" fix (security review
finding 2, round 3, coordinator ruling): round 2 made a duplicate
top-level "model" key WITHIN the 64KiB peek window fail closed, but a
client can still pad the body so a second "model" sits just PAST
maxModelPeekBytes -- `{"model":"cheap", <64KiB padding>,
"model":"expensive"}`. The peek reports "cheap" (the only key it saw),
the body is forwarded byte-for-byte, and the upstream (last-wins on
every mainstream JSON parser) executes "expensive".
scanTopLevelModel now also reports whether it actually confirmed the
top-level object's closing '}' within the scanned window (closed).
peekPassthroughModel combines that with its own knowledge of whether the
read hit the byte cap (more of the body remains unread) to report
truncated: true only when both hold, meaning a later duplicate cannot be
ruled out. handlePassthrough denies (403, a message distinct from "model
could not be determined") whenever truncated is true, checked before
even looking at the found model -- a found model under truncation can
never be trusted.
Scoped exactly as ruled: only affects a group with a non-empty Models
list (grp.hasModelRestriction) whose body is large enough to hit
maxModelPeekBytes -- an unrestricted group never calls
peekPassthroughModel at all, unchanged from round 2 and pinned by a new
test. No live group is model-restricted today. The unified /v1/* route
remains the supported path for a legitimately large restricted-group
body.
Tests: the coordinator's own exploit body end-to-end (403, upstream
never called); the general non-duplicate case (any restricted-group body
that never closes within the window denies, not just a provably
duplicate one); the unrestricted-group-unaffected case explicitly, using
the identical exploit body; and closed's own truncation-tracking table
against scanTopLevelModel directly. One pre-existing test
(TestHandlePassthrough_BodyIntegrity_ByteIdentical_LargeBody) used a
restricted group incidentally while testing an unrelated property (byte-
for-byte forwarding) -- switched to an unrestricted group so it tests
only what its name says.
|
||
|
lukaszraczylo
|
f736b231a3 |
fix(routes): admit before decode, bound in-flight bodies, narrow unified cap
Close a HIGH-severity OOM path on the unified and media JSON routes (security review finding 1). Three parts: - checkAndCount now runs via a new admitRequest helper BEFORE the request body is read at all, on both routes_unified.go's runUnified and every routes_media.go handler. The rate-limit scopes need only the already-resolved user/group, so a caller already over their requestsPerMinute is refused without paying for the read+decode of a body that was always going to be discarded. Measured: an encoding/json decode into map[string]any costs ~11x the wire body in live heap, so the prior ordering let an already-limited caller burn ~110MiB per request before ever seeing the 429. - A new bounded, buffered-channel semaphore (Gateway.bodyAdmission, mirroring limits.go's spawnTokens shape) caps how many unified/media requests may concurrently hold a decoded body in memory. Acquired non-blockingly at each route's entry via acquireBodyAdmission; on exhaustion, returns 503 with Retry-After rather than blocking. Sized by defaultBodyAdmissionCap (GOMAXPROCS * 8, clamped to [16, 128]); Config.MaxInFlightBodyRequests, when set, always overrides the self-tuned default. - The unified chat/embeddings routes now decode against a new, narrower maxUnifiedRequestBytes (4MiB) instead of the shared maxRequestBytes (10MiB), which the media JSON routes, audio/transcriptions' multipart cap, and the MCP federation JSON-RPC routes keep unchanged. 1MiB (the audit's own first suggestion) was evaluated and rejected: this package documents and exercises inline base64 image_url content parts in chat messages (translate_anthropic.go, translate_gemini.go), and a single moderately-sized photo commonly exceeds 1MiB once base64-encoded. 4MiB preserves that documented single-inline-image case while still cutting worst-case per-request decode amplification from ~110MiB to ~44MiB. routes_media.go's resolveMediaRequest is split: model resolution stays (renamed resolveMediaModel), the limit check moves into the shared admitRequest. Tests: admission-before-decode is asserted directly (a 429'd request's body is never Read), semaphore exhaustion is asserted deterministically by filling the channel directly, the self-tune/override contract and its clamp bounds are pinned, and a zero-config run of ordinary traffic is asserted to see no new 503s. A dedicated test proves the chosen 4MiB cap still accepts a realistic ~1.5MiB inline image — the case 1MiB would have broken. |
||
|
lukaszraczylo
|
bf33732123 |
perf(registry): cache the encoded /v1/models response per group
GET /v1/models cost 15.9ms, 21.3MB, 552,884 allocs per call at the
live 1,033-model catalog (registry.go's listFor plus routes_unified.go's
json.Encoder), uncached — and OpenAI SDK clients poll this endpoint per
session, so an unchanged catalog was being re-walked and re-marshaled
on every request.
Add modelRegistry.modelsJSON(grp), caching the fully encoded
{object:list,data:[...]} body per (*group, modelsGen). modelsGen
is an atomic counter bumped by a new modelRegistry.finishRefresh
wrapper around providerState.finishRefresh — the only two places that
ever mutate a provider's discovered set or discovery-captured metadata
(warmFill, refreshProvider) already funnel through providerState.
finishRefresh once per refresh cycle, so wrapping it is sufficient to
invalidate the cache on every possible change, including a
metadata-only update on an otherwise-failed refresh (captureModelMetadata
runs unconditionally alongside finishRefresh in both call sites).
Bumped unconditionally rather than only on success, since a failed
listModels fetch can still pair with a successful metadata capture.
Cached per *group pointer: authStore builds every group once, at
construction, and never rebuilds or replaces that map afterward — a
config reload constructs an entirely new Gateway (and modelRegistry)
with its own empty cache, so a *group key can never alias a different
group's catalog either within one registry's lifetime or across a
reload.
routes_unified.go's handleModels call site shrinks to a single write of
modelsJSON's result, replacing the previous inline listFor+json.Encode;
routes_unified.go is otherwise untouched (owned by a sibling change in
this same effort).
|
||
|
lukaszraczylo
|
90eb58171e |
perf(redis_store): send EXPIRE only once per key per process lifetime
31 of 74 commands per request in the interpreted-harness audit were EXPIRE, emitted unconditionally after every INCRBY. Deleting EXPIRE entirely (upper bound) measured 74->43 cmds/req, p50 867.9->697.9us (-19.6%), conc=16 throughput 1,477->2,336 req/s (+58%), allocs -26%. Route chosen: a bounded first-touch-per-key cache (expireOnce), not EVALSHA+Lua. windowKey (limits.go) already embeds each counter's own time bucket in its key string, and every TTL callers pass is deliberately far longer than its window's own natural length purely as a stale-key GC margin (limits.go's own doc comment on dayWindowTTL/monthWindowTTL/etc.), not a sliding-window mechanism — so once EXPIRE has been sent for an exact key, every further INCRBY against that SAME key happens well within its own bucket's window, comfortably inside the TTL's margin. Skipping the redundant resend cannot leave a key un-expiring: a bucket's key simply stops being written once its window rolls over (a new bucket string takes its place). expireOnce is bounded (expireOnceCacheMax=4096, FIFO eviction) so a long-lived process's ever-rolling buckets cannot grow it unboundedly; an evicted key just gets one redundant-but-harmless EXPIRE again, exactly as if never touched before. incrBy/incrMulti/incrAndGetMulti now build a variable-stride command list (each entry contributes its own INCRBY, plus EXPIRE only when expireOnce(key) is true) with an explicit incrReplyIdx/base tracking each entry's actual reply position — no longer a fixed i*2 stride, since a skipped EXPIRE shifts every later command left by one. EVALSHA+Lua was rejected as the shippable route here: it would rewrite every command-building and reply-index path, requires NOSCRIPT fallback handling, and every existing wire-level test (dozens of INCRBY+EXPIRE-scripted fakeRESPServer cases) would need rebuilding around SHA1 digests for no additional correctness benefit over the simpler first-touch cache. |
||
|
lukaszraczylo
|
2309a59b40 |
perf(resp): pool redis connections instead of one mutex-guarded conn
respClient held a single net.Conn behind one mutex, serializing every caller's whole round trip. Interpreted-harness audit measured this as the throughput ceiling: 1,458 req/s Redis-backed vs 7,811 req/s on memoryStore for the same workload, latency scaling near-linearly with concurrency (869us@conc=1 -> 10.81ms@conc=16). Replace the single connection with a pool of independently usable connections (respConn slots in a buffered channel), each acquired for one caller's whole round trip and returned via a deferred send back onto the pool. A broken connection is discarded (closeConn) but its slot always returns to the pool, so pool size never shrinks after an error. Default size is GOMAXPROCS clamped to [4, 8] (defaultRespPoolSize); RedisConfig.PoolSize lets an operator override it explicitly, per the house rule that an explicit override always wins over self-tuning. Preserves exactly: the 2s respCallTimeout call deadline, reconnect- once-on-error semantics, nested-array rejection, the RESP size bounds, and every existing pipelining contract (incrMulti, incrAndGetMulti, getMulti, cache get/set) — nothing outside resp.go references a connection field directly. Interpreted-harness prototype (variants/resp_pool8.go, pool=8) measured at a realistic 200us Redis RTT, conc=16: p50 24.06ms -> 3.24ms (-86%), throughput 663 -> 4,670 req/s (+599%, +-1% over 3 reps); a pool=1 control matched HEAD exactly, confirming the win is parallelism, not refactor noise. cache_test.go and llmgateway_test.go: adjust white-box tests that issue multiple sequential calls against one respClient scripted with runFakeRESPServer's single-connection model — force pool size 1 there (newRESPClientPool) so the test still pins one connection's exact wire sequence; production callers are unaffected. |
||
|
lukaszraczylo
|
df39ad3739 |
chore(admin): regenerate baked admin assets
Regenerated via `make admin-ui` against the new webui/generate.mjs chunking. Decoded asset bytes are byte-identical to the prior baked file (SHA-256 over adminIndexHTML + every adminAssets[name].body, sorted by name: 7bf96c68a64d26586057c714ba8b6ee393ad361a47ab1acb21d1d260955d3edd before and after — verified with a throwaway test against both generator outputs on the same webui/dist/ build). contentType strings and asset map keys are unchanged. Measured under Yaegi (github.com/traefik/yaegi, the interpreter this plugin actually runs under) with a standalone import-timing harness pointed at this tree: package import 233.8ms -> 39.8ms (median of 3 runs each), heap after import 32.0MB -> 22.8MB. |
||
|
lukaszraczylo
|
e5c3cd4b97 |
perf(admin): stop wrapping baked assets in a 7500-literal + chain
webui/generate.mjs split each admin-panel asset's base64 into 120-char Go string literals joined with `+` (~7,550 literals, ~7,538 `+` ops across index.html + the JS/CSS bundle). A compiled Go binary folds that at compile time for free, but this plugin loads under Yaegi (github.com/traefik/yaegi), which folds a chain of N literal `+` operations at import time by repeatedly reallocating and copying the whole already-folded prefix: O(N^2). Measured: ~186ms of a ~223-240ms package import was this folding alone, versus ~3-4ms to decode the asset bytes. Replace the 120-char LINE_WIDTH with a 1,000,000-char CHUNK_CHARS cap. Every asset this panel ships today (largest ~516,000 base64 chars) fits in a single literal, so goBase64Expr now emits one decodeAdminAsset call with zero `+` folds per asset. The cap still bounds a future, much larger bundle to a small number of large chunks rather than one unbounded line; single-line Go string literals in this size class were verified against gofmt/go vet/go build before picking the constant. No behavior change: decoded asset bytes, contentType strings, and the asset map keys are unaffected by how the base64 is chunked into source literals — see the following regen commit for the byte-identity proof. |
||
|
lukaszraczylo
|
e06d8bf5cd |
fix(passthrough): fail closed on a duplicate top-level model key
RFC 8259 permits duplicate names and every mainstream upstream parser
resolves them last-wins, while a passthrough body is forwarded verbatim.
Reporting the first occurrence let a model-restricted caller authorize
{"model":"allowed","model":"expensive"} against the allowed one and
execute the expensive one. Refuse to answer instead, matching
extractMultipartModel's existing duplicate rule.
Found by the security audit; a padded-duplicate variant that hides the
second key beyond the 64KiB peek window is tracked separately.
|
||
|
lukaszraczylo
|
e57a97d63e |
fix(passthrough): close Content-Type bypass and matcher-parity bug
Independent review measured two real gaps in the round-1 model
enforcement and fixed both here, plus documentation and comment
regressions the round introduced.
MUST FIX: peekPassthroughModel's Content-Type gate was an ALLOW-list
("application/json" or skip") and fully client-controlled. The
reviewer sent "Content-Type: text/plain" with {"model":"EXPENSIVE"}
and got a 200 from a group scoped to a cheap model. This commit
replaces the whole gating approach:
1. A group with an empty Models list has nothing for allowsModel to
reject, so the request body is never read at all
(group.hasModelRestriction). This is the overwhelming majority of
traffic, including the operator's own "home" group, and removes
the memory-buffering concern entirely for it.
2. For a group WITH a restricted Models list, the Content-Type gate
is now a SKIP-list of genuinely binary types (multipart/, audio/,
image/, video/, application/octet-stream). Every other
Content-Type, including "text/plain" or none at all, is
inspected. The attacker no longer controls whether the check
runs.
3. The peek reads at most 64KiB (maxModelPeekBytes), restored via
io.MultiReader so the remainder streams straight to the upstream
instead of a second full buffer. A bounded JSON token walk
(scanTopLevelModel) finds a top-level "model" key even when the
rest of the body is truncated past the cap.
4. A restricted group whose peek window never finds "model" is
denied (403), fail closed. This also means a Gemini passthrough
request (its model lives in the URL, never the body) is always
denied for a model-restricted group; documented in
maxModelPeekBytes' own comment, the handler's doc comment, and
the README.
Matcher parity: the round-1 version glued providerName+"/" onto
whatever the body sent unconditionally, so an already-prefixed body
("openai/x") produced "openai/openai/x" and false-403'd a legitimate
client. allowsPassthroughModel now reuses modelRegistry.
splitConfiguredProvider — the same rule resolve/resolveAgainst apply
to a client-given id — to strip a leading segment only when it names
an actually configured provider, before generating candidates.
Also in this commit:
- Restored ~26 lines of doc comments fieldalignment -fix silently
dropped when it reordered the group struct (auth.go: cache,
cacheTTL) and ProviderConfig (llmgateway.go: MetadataPath, and the
Passthrough field's own new comment). Verified nothing else was
reordered by re-running fieldalignment: 0 issues.
- Documented the path.Match footgun a non-empty PassthroughPaths
inherits: "*" does not cross "/", so ["*"] denies almost all real
multi-segment passthrough traffic rather than allowing it. Empty is
the actual allow-all value. Documented on the config field, the
allowsPassthroughPath method, and the handler's own doc comment.
- Renamed the local "model" that shadowed the outer request-model
variable near extractPassthroughUsage's call site to "respModel" —
the new gating already scopes the outer one to a narrower block, so
this was no longer a live shadow, but the rename removes any future
ambiguity between the request's model and the response's.
- README: rewrote the two "Unified vs. passthrough" table cells that
described the OPPOSITE of this feature's actual behavior, added a
"Native passthrough model authorization" section, and added
providers.<name>.passthrough / groups.<name>.passthroughPaths rows
to the config tables.
New tests: the Content-Type-bypass regression (text/plain, absent,
form-urlencoded, all now 403), the matcher-parity case (already-
prefixed body allowed), fail-closed on a binary body for a restricted
group, skip-entirely for an unrestricted group, byte-identical body
forwarding for a body larger than the peek cap and for a
ContentLength<0 (chunked) request, and a scanTopLevelModel unit table
covering truncation before/after the model field, nested values, and
malformed input.
Gate re-run in full after these changes: gofmt, go vet, fieldalignment,
golangci-lint (0 issues), go test -race (ok), make yaegi-check (OK),
make integration (ok, real Traefik + Redis). Re-ran the interpreted
Yaegi harness: RTT/req unchanged at 3.00, cmds/req unchanged at 74.00.
|
||
|
lukaszraczylo
|
311dd20a8e |
fix(limits): correct dangling comments and redisStore edge cases
Independent review round approved the fusion itself (11 to 3 RTT/req, precedence verified branch by branch, latch/fail-open checked under Yaegi against a dead Redis). This commit addresses the follow-up items raised against that same change. Six doc comments in limits.go still named evaluateScope, budgetViolation, or tokenBudgetViolation as if they were live functions to consult. All three were removed by the fusion commit; the comments now point at checkAndCount and buildBudgetProbes, where the same logic lives today. One further comment claimed checkAndCount still slices a "storeIncrMulti result" — it now slices storeIncrAndGetMulti's own incrVals. TestCheckAndCount_FusedViolationPrecedence's doc comment claimed every subtest engineers two simultaneous violation conditions. The seventh engineers only one (the group violates; the user carries no limits at all) — it pins a different claim, that a later scope is still evaluated when an earlier one has nothing to check. The comment now says so. redisStore.incrAndGetMulti gained the same len()==0 guard incrMulti already has, and a table-driven test proves a RESP error or a non-integer value on a read key fails the whole call, matching respClient.getBatch's own equivalent table. Re-ran the interpreted Yaegi harness after these changes: RTT/req stays at 3.00, cmds/req stays at 74.00 — the fusion's throughput win is unaffected by this round's fixes. |
||
|
lukaszraczylo
|
e3ef4084d2 |
feat(passthrough): add model, path, and header authorization hardening
handlePassthrough checked only the provider allowlist, never the model or the path. A tenant scoped to one cheap model could reach a provider's entire native API (files, batches, fine-tuning) on the operator's key, and client identity headers reached keyless in-cluster MCP/A2A targets unfiltered. This change closes both gaps, additively: every new field defaults to reproducing prior behavior exactly. Model enforcement: when a passthrough JSON body carries a top-level "model" field, handlePassthrough checks it against the group's model glob, in both the bare and provider-prefixed form (the same dual-candidate matcher the unified route's resolveAgainst already applies). A body with no inspectable model (Gemini's URL-embedded model, a multipart upload, a non-JSON body) falls back to provider-only authorization, unchanged from before this round. Per-provider toggle: ProviderConfig.Passthrough, nil or true by default, lets an operator disable a provider's native passthrough route. A disabled provider now reports the ordinary unknown-route 404, checked in ServeHTTP's own route gate, before auth even runs. Per-group path allowlist: GroupConfig.PassthroughPaths, empty by default (matching every other allow-list field's empty-means-all semantics), restricts which native rest path a group may address. Dangerous-header stripping: proxyUpstream now additionally strips Forwarded, X-Forwarded-*, X-Auth-*, X-Remote-User, X-Remote-Groups, and Cookie on both the native-passthrough and MCP/A2A target-proxy paths, plus OpenAI-Organization, OpenAI-Project, and anthropic-beta on native passthrough only. This is an additive denylist expansion, not an allowlist inversion; ordinary headers still pass through. A dedicated test asserts a config with none of the new fields set behaves identically to before this change. The operator's home group (providers/models/mcpServers/agents/passthroughPaths all empty) stays fully unrestricted, and macstudio-rerank/parakeet-mlx/piper-tts stay reachable through native passthrough by default. |
||
|
lukaszraczylo
|
a418814b34 |
perf(limits): fuse admission reads into one round trip
checkAndCount already batched its request-counter increments into one pipeline. evaluateScope then read every scope's token and cost budgets through up to four further serial round trips per scope. A request with a fully-limited user and group scope paid nine round trips before the upstream call started. This change fuses every budget read into the SAME pipeline as the increments, through a new incrAndGetMulti method on counterStore. The reads are safe to fuse because they touch disjoint keys (req:* versus tokin:*/tokout:*/cost:*), and tokens and cost are only ever written by account() after the response completes, never by checkAndCount itself. Violation precedence stays identical to the pre-fusion implementation: a user's own violation still wins over their group's, and within one scope the order stays requests-per-minute, requests-per-day, tokens-per-day, tokens-per-month, cost-per-day, cost-per-month. New table-driven tests pin this precedence directly, engineering two simultaneous violation conditions per case and asserting only the higher-precedence one is reported. evaluateScope, budgetViolation, and tokenBudgetViolation are removed; buildBudgetProbes plus a loop inline the same logic into checkAndCount, which is what lets their reads share the fused round trip. Measured on the interpreted Yaegi harness (yaegi v0.16.1, the runtime this plugin actually ships under): RTT/req for a fully-limited chat request drops from 10.99 to 3.00, with cmds/req unchanged at 74 (this is a round-trip fix, not a command-count or allocation change). Single-request throughput rises from 112 to 335 req/s; at concurrency 16, from 121 to 380 req/s. counterStore.incrAndGetMulti is a required interface method, not an optional capability behind a type assertion: Yaegi can silently return ok=false when a comma-ok assertion checks a compiled concrete value against a plugin-declared interface (Trap 4, documented in matchesSentinel's own doc comment). A plain interface method call carries no such risk and is the same mechanism incrMulti already uses in production. |
||
|
lukaszraczylo
|
2d10509e20 |
docs: render correctly on the Traefik plugin catalog
The catalog uses a plain CommonMark renderer, not GitHub's: the "[!WARNING]" alert syntax printed literally and the mermaid block rendered as a wall of diagram source. Use a portable bold blockquote and fold the diagram into <details> so plain renderers collapse it while GitHub still draws it. |
||
|
lukaszraczylo
|
1875ee7b8b |
fix(registry): defer context cancels on the refresh panic path
A panic in listModels skipped the inline cancel, holding the timeout timer until it fired. Review follow-up from the v0.2.3 final verdict. |
||
|
lukaszraczylo
|
0a37fa1b47 |
chore(admin): regenerate baked admin assets
make admin-ui: vue-tsc -b && vite build, then webui/generate.mjs against webui/dist/, gofmt -w'd. Reflects the review-round webui fixes: the compact-format precision corrections, the ARIA-valid hover-text components, and the ProvidersView.vue polish. |
||
|
lukaszraczylo
|
171f6d04c9 |
docs(readme): document model metadata (review SHOULD-7)
New 'Model metadata' section: the context/cost layering order, the modelMeta config override example (mirrors the operator's own cluster snippet), alias inheritance, the ":free" naming convention, the metadataPath discovery capture (including the loaded-vs-max-context volatility note — a discovery refresh can legitimately report a different context window across reloads), the pricing-sync/built-in table workflow, and the two GET /v1/models extension fields with a worked example. Config reference tables gain a ModelMetaConfig subsection and modelMeta/metadataPath rows on Config/ProviderConfig; the Admin section notes the per-provider and per-alias modelMeta fields on GET /admin/api/overview. |
||
|
lukaszraczylo
|
fb1b2a36ff |
test(yaegi-check): exercise the builtin table lookup (review SHOULD-6)
The harness previously only ever proved resolveModelMeta's CONFIG- OVERRIDE layer runs interpreted (testDataWantModel carries an explicit modelMeta entry). It never exercised an interpreted LOOKUP into the real, 236-entry builtinModelMetaTable map literal at all. Adds "gpt-4o" to the harness's openai provider with NO modelMeta override of its own, so its GET /v1/models context_window can only come from the builtin-table branch. Its expected value is read directly out of pricing_data_gen.go at harness run time (readBuiltinContextTokens), not hardcoded, so this assertion self-updates across a `make pricing-sync` regeneration instead of pinning a number that could legitimately drift when upstream LiteLLM data changes. Also switched both existing GET /v1/models assertions from raw strings.Contains substring matching to a structured JSON decode keyed by model id — a bare substring check could not tell which model entry a given context_window value actually belonged to, which now matters with two entries in play. |
||
|
lukaszraczylo
|
9b60c0afae |
fix(webui): ARIA-valid hover text, alias metadata guard, context-chip polish
CompactNumber.vue and ProviderRateBadge.vue both put aria-label on a
plain <span> — axe's aria-prohibited-attr rule: a <span>'s implicit
ARIA role ("generic") does not support accessible-name computation at
all, so the attribute was simply invalid there, not just discouraged.
Both now use the visible-text-hidden-plus-sr-only-full-text pattern
instead: the compact/label text is marked aria-hidden="true", and a
sr-only sibling span carries the exact value/detail for assistive tech.
title stays on the outer element for mouse hover, unchanged. Folded
review minor.
ProvidersView.vue: the alias table's ModelChip cell read
row.original.modelMeta.contextTokens without a guard, unlike the
provider path's own modelMetaFor fallback — a response from a server
build older than this field (version skew) would throw instead of just
omitting the hover detail. Now optional-chained to match. The visible
context chip beside each model gains title="context window" and
text-xs (was an arbitrary text-[10px]), matching the rest of the panel's
muted-label sizing convention. Folded review minors.
|
||
|
lukaszraczylo
|
5e3cf919d9 |
fix(webui): compact-format precision bugs (review IMPORTANT-2/3)
formatCompactCount: Math.floor(scaled*factor)/factor under-reported by one unit whenever the intermediate scaled=abs/divisor value was not exactly representable in binary floating point (8_700_000 -> "8.69M", not "8.7M" — 8.7 itself has no exact double representation, so the original two-step division/floor landed on 869 instead of 870). Reordered to floor(abs*factor/divisor) — multiplying the original INTEGER first keeps every intermediate value exact for any realistic count, since abs*factor stays within Number.MAX_SAFE_INTEGER. New rows: 8_700_000 -> "8.7M", 1_130_000 -> "1.13M". (review IMPORTANT-2) formatContextWindow: always divided by 1024 (binary-K), which is right for a genuinely binary context size (262144 -> "256k") but wrong for a decimal, marketing-style round count (128000 -> "125k", an odd number nobody would recognize as "128K"). A plain "tokens % 1024 == 0" check cannot separate these — 128000 is ALSO an exact multiple of 1024 (125 x 1024) despite not being a power of two. The actual, consistent discriminator is POWER OF TWO: 262144 = 2^18, 128000 = 2^10 x 5^3 (not a power of two). Binary-K/M now applies only when tokens is an exact power of two; decimal-K/M otherwise. Fixed rows: 128000 -> "128k" (was "125k"), 1_000_000 -> "1M" (was "977k"), 2_000_000 -> "2M" (was "1953k"); 262144 -> "256k" and 32768 -> "32k" unchanged. (review IMPORTANT-3) |
||
|
lukaszraczylo
|
e3996d88bf |
fix(pricing): content-digest snapshot instead of a wall-clock date
The generated pricing_data_gen.go header used to stamp today's date on every `make pricing-sync` run, so a re-run against genuinely unchanged upstream data still touched the file and produced a spurious daily diff with nothing real to review. The header now records a short SHA-256 digest of the FETCHED bytes instead — identical upstream data always reproduces a byte-identical generated file. Folded review minor. |
||
|
lukaszraczylo
|
962f5eeeed |
fix(providers): validate metadataPath has a leading slash
fetchModelMetadata (provider_openai.go) builds its request URL by plain string concatenation, a.baseURL+a.metadataPath. A metadataPath missing its leading slash would silently glue onto baseURL's own path with no separator (e.g. "http://host:1234api/v0/models"), a malformed URL that only ever surfaced as a confusing runtime fetch failure. buildAdapters now rejects a non-empty metadataPath without a leading "/" at construction, naming the provider, the same fail-fast treatment every other malformed config value already gets. Empty (the default) is always valid. Folded review minor. |
||
|
lukaszraczylo
|
82dcaa5309 |
fix(registry): metadata capture gets its own timeout + panic isolation
warmFill/refreshProvider used to reuse listModels' own context for the subsequent captureModelMetadata call — a listModels attempt that took real, non-trivial time (or timed out outright) guaranteed the metadata fetch an already-spent, near-expired context, producing a spurious deadline error and log line every warm fill or refresh for an otherwise healthy provider. Both call sites now give captureModelMetadata its own fresh context.WithTimeout budget, computed after listModels returns. (review SHOULD-4) captureModelMetadata also gains its own deferred recover, isolating a panic inside fetchModelMetadata (or a future modelMetadataFetcher implementation) from refreshProvider's OUTER recover — which would otherwise overwrite the (already-nil, already-correct) err variable from a successful listModels call and cause finishRefresh to discard a freshly fetched, genuinely good model list over an unrelated bug in the metadata path. "Metadata must never fail discovery" is now enforced on the panic path, not just the plain-error-return one already handled. (review SHOULD-5) New regression tests: a fresh-timeout-budget check (a slow simulated listModels must not shrink the metadata fetch's own deadline) and a panic-in-fetchModelMetadata check (discovery must still succeed and record both models). |
||
|
lukaszraczylo
|
933d869e32 |
fix(modelmeta): free-tier suffix layer (review IMPORTANT-1)
A ":free"-suffixed model id (a real provider naming convention, e.g. OpenRouter's own live API) is free by naming convention alone, independent of whatever the built-in table does or does not track for that exact id. resolveModelMeta's cost switch gains a new case, ranked above the built-in table and below an operator's own config override — so a provider's free-tier naming is never shadowed by a stale or coincidental paid built-in price under the identical key, and an operator can still override it explicitly if they choose. Context is untouched by this rule: it keeps resolving through the normal config/ discovery/builtin/absent layers, never inherited from an unrelated entry. Also makes resolveModelMeta's own Free case assign 0, 0, true explicitly (matching resolveAliasModelMeta's existing style) instead of relying on the zero-value default — defensive consistency, folded review minor. Independently reviewed and reproduced against the same live LiteLLM fetch: zero ":free"-suffixed keys exist anywhere in the 3111-entry upstream file, confirming the earlier grounding note in the v0.23 report. This layer is a general resolution rule for a naming convention provider APIs use in live discovery, not something the pruned build-time table needs to carry any entries for. |
||
|
lukaszraczylo
|
002ee3b143 |
chore(admin): regenerate baked admin assets
make admin-ui: vue-tsc -b && vite build, then webui/generate.mjs against webui/dist/, gofmt -w'd. Reflects every webui/ change in this branch: feature v0.23's metadata hover/chips and the compact-count formatting addition. |
||
|
lukaszraczylo
|
040ff40ae0 |
feat(webui): compact usage-count formatting
Operator scope addition: raw token/request counts reaching millions/
billions were unreadable as bare digits. formatCompactCount (lib/
format.ts, shipped in the previous commit alongside this feature's own
helpers) renders an SI-style label: below 10,000 the exact integer with
thousands separators, at or above it three significant figures with a
k/M/B suffix ("12.3k", "456k", "1.23M", "45.6M", "1.23B") —
every digit past the third significant one is FLOORED, never rounded,
the same honesty rule formatRatePercent already applies to the
success-rate badge: a count of 999,950 must read "999k", never
"1,000k", since it has not actually crossed that boundary.
CompactNumber.vue is the one shared, reusable presentational component
(component-first: no per-call-site wrapper markup) — renders the
compact form as text, with the exact value on both title and aria-label,
mirroring ProviderRateBadge.vue/ModelChip.vue's own hover-parity
pattern.
Applied everywhere a count renders: usage-columns.ts's numericColumn
(reqMin/reqDay/tokIn-out per day and month — cost columns explicitly
opt out via a new compact:false argument, since formatCost already
shows full precision), target-columns.ts's MCP/agent request counters,
UsageView.vue's Total card and group-accordion trigger/detail stat
grid, and UsageChart.vue's Chart.js y-axis ticks (compact only — no
room for the raw figure on an axis label) and tooltip labels (compact
form WITH the precise raw value alongside, e.g. "Tokens in: 1.23M
(1,234,567)").
Sorting is unaffected by construction: every numericColumn's accessorFn
still returns the raw number (what getSortedRowModel actually sorts by)
regardless of what the cell renders — usage-columns.spec.ts pins this
directly, including a sort-order assertion that would fail under a
lexicographic (string) sort but passes under the numeric one.
|
||
|
lukaszraczylo
|
eac0862d68 |
feat(admin,webui): metadata exposure
admin.go: adminProviderView gains ModelMeta (map[string]adminModelMetaView,
keyed by upstream model id) and adminAliasView gains its own ModelMeta —
both built via g.registry.resolveMetaFor/resolveMetaForAliasName
(feature v0.23). adminModelMetaView uses pointer fields
(*int/*float64), not plain zero values: a nil pointer means unknown, a
non-nil pointer (even one pointing at 0) means known, so a free model's
real zero cost never gets confused with an absent one over JSON.
resolveMetaForAliasName resolves an alias's metadata for this
unrestricted admin view via an internal &group{} (GroupConfig's own
documented empty-means-all semantics) — buildAdminOverview already
shows every provider/alias unconditionally, with no group filtering
anywhere else in its response either.
webui/src/types/api.ts: AdminModelMetaView mirrors the Go shape
(optional numbers, undefined means unknown); AdminProviderView.modelMeta
and AdminAliasView.modelMeta wire it through.
webui/src/lib/format.ts: formatContextWindow (binary-K "256k"-style),
formatModelCostHover ("in $X / out $Y per MTok", or "free"), and
their shared formatUsdPerMTok building block.
ModelChip.vue (operator scope refinement): every chip — the accordion's
model chips, the alias table's chips, a group's usage-detail chips —
gains optional contextTokens/inputPerMTokUsd/outputPerMTokUsd props.
When known, they append a resolved-metadata phrase to the existing
copy-state hover text, bound to BOTH title and aria-label (the same
mouse-vs-keyboard/screen-reader parity ProviderRateBadge.vue already
applies); when nothing is known, the hover is unchanged from before this
feature. An alias's chip carries its INHERITED metadata, matching the
alias-inheritance rule the backend already applies.
ProvidersView.vue: the accordion's model list gains a visible, muted
context-only chip ("256k") beside each ModelChip — cost stays
hover-only on the chip itself, per the operator's own "chips +
hover would be redundant on price" resolution.
|
||
|
lukaszraczylo
|
0c61296c73 |
feat(registry): model metadata capture and resolution
Config gets two new surfaces (llmgateway.go): Config.ModelMeta (map[string]*ModelMetaConfig, keyed by exact "provider/model" or a bare model id) for operator overrides, and ProviderConfig.MetadataPath for openai-type providers whose discovery should also capture per-model context (LM Studio's native /api/v0/models shape). modelmeta.go adds the layering itself: resolveModelMeta applies config override > discovered context > builtin table > absent for context, and config override/free > builtin > absent for cost (discovery never reports cost). resolveAliasModelMeta implements alias inheritance: an alias inherits its target's fully resolved metadata unless the alias itself has its own modelMeta entry, applied per field. validateModelMeta rejects negative values and a free:true/non-zero-cost contradiction at construction. registry.go wires this into the request path: providerState gains a discoveredContext map (stale-while-error, mirroring discovered's own convention) with contextFor/setDiscoveredContext; a new optional modelMetadataFetcher interface, type-asserted (interpreted-to-interpreted, documented as yaegi-safe against the Trap-4 restriction) rather than added to providerAdapter, lets warmFill/refreshProvider capture metadata alongside listModels without touching anthropic/gemini adapters at all. modelObject/listFor now attach resolved context_window/pricing to every GET /v1/models entry, including aliases (resolveMetaFor/ resolveMetaForAlias/resolveMetaForAliasName). provider_openai.go implements modelMetadataFetcher: fetchModelMetadata reads ProviderConfig.MetadataPath, preferring loaded_context_length over max_context_length when the LM Studio response reports both. providers.go wires MetadataPath onto the adapter in buildAdapters. tools/yaegi-check/main.go adds one cheap modelMeta config-override assertion to the existing attempt-accounting harness, proving resolveModelMeta's config-override layer and the context_window/pricing extension fields both run correctly under the real Yaegi interpreter. |
||
|
lukaszraczylo
|
e8398e6d49 |
feat(pricing): litellm sync generator + table
Add tools/pricing-sync: a standalone, compiled Go tool that fetches
LiteLLM's model_prices_and_context_window.json at build time and
regenerates pricing_data_gen.go, a plain map[string]builtinModelMeta
literal keyed by bare model id (context window, input/output cost in
micro-USD per million tokens).
Pruning rule (documented on the tool's own package doc comment): keep
only entries whose litellm_provider matches this gateway's own provider
types (openai, anthropic, deepseek, xai, minimax, cerebras, dashscope
for qwen/alibaba, and moonshot for Kimi models) and whose mode is chat
or embedding, dropping everything else — a 3111-entry table is a real
startup cost under Yaegi interpretation, so the generated table stays at
236 entries. Bare-id collisions across LiteLLM keys resolve
deterministically: a bare key beats a provider-prefixed one, then
provider priority, then lexicographic order.
Naming bridges: a small, data-driven table (namingBridges) duplicates an
entry under a second bare id when a provider's own live API disagrees
with LiteLLM's naming for the identical model — seeded with MiniMax's
lightning/highspeed dual naming, verified against a live deployment.
Coverage-audit follow-up (verified against a live fetch of the same
source, 2026-08): moonshot's own "litellm_provider" tag carries 23
real Kimi entries (kimi-k2.5, kimi-k3, ...), added to the allowlist as a
first-class provider rather than a per-id bridge, since the table is
already keyed by bare id regardless of provider. Two claims from a
coverage-audit request could NOT be verified against this same fetched
file and were deliberately NOT implemented: no key anywhere in the
3111-entry source ends in ":free" (an openrouter free-tier naming
convention the request assumed exists in this file), and no
"qwen/"-prefixed key exists at all — see the report for the full
grounding note. Also confirmed already-correct by design, no change
needed: audio/responses-mode OpenAI entries ("codex-mini-latest" and
friends) are excluded by the existing mode allowlist, not by any
provider special-case; image/video media models are excluded the same
way.
builtinModelMeta itself lives in its own file (builtin_model_meta.go),
separate from modelmeta.go's resolution logic, so this commit's table
and generator compile standalone.
make pricing-sync regenerates the table; add its gitignore entry for the
tool's own compiled binary.
|
||
|
lukaszraczylo
|
909dba5a44 |
fix(limits): recover and log in the telemetry spawn, broaden the yaegi trap note
The spawned write gains the registry-precedent recover (an unrecovered panic off any request goroutine crashes the whole Traefik process) and rate-limited drop logging so a stressed store is distinguishable from a healthy provider. The matchesSentinel doc now records the instrumented finding: ANY compiled concrete value fails a comma-ok assertion against ANY interpreter-declared interface, method count irrelevant, silently — plus the compiled-origin trip-wire that keeps errors.Is safe here. |
||
|
lukaszraczylo
|
21f51dc282 |
docs(readme): note bounded fire-and-forget writes for provider counters
Re-read the provider/model success-rate paragraph against the FIXED
deadline behavior (v0.22 review round, round 2) per instruction — the
claim itself ("a real deadline counts as a failure") was already
accurate and needed no correction; the multi-wrap bug it was allegedly
describing never surfaced through the README, since the README
describes intended behavior, not the wrap-depth implementation detail
that broke it. Added: FOLDED-1's bound (64 in flight per process,
lossy-under-pressure by design) on the fire-and-forget write this
paragraph already described, and an explicit note that enforcement never
goes through this path.
|
||
|
lukaszraczylo
|
7eac56cb46 |
docs(webui): fix stale percent in minuteRateTitle doc example
SHOULD-B (v0.22 review round, round 2): the doc comment still showed "(92%)" for 12 attempts/1 failure — stale against the floor-not-round fix (formatRatePercent) landed earlier in this same file, which the spec's own test asserts as 91%. No functional change; comment only. No admin-asset regen needed — verified byte-identical dist/ output before and after (a doc comment does not reach the minified bundle). |
||
|
lukaszraczylo
|
eddcbe35bd |
fix(providers): multi-wrap deadline detection was dead on 3 of 4 routes
BLOCKER (v0.22 review round, round 2): matchesSentinel's hand-rolled
Unwrap walk (round 1's SHOULD-1 fix) stopped dead the moment it hit
providers.go's own multi-%w wrapping. fmt.Errorf with more than one %w
verb builds a *fmt.wrapErrors implementing Unwrap() []error, not
Unwrap() error — a comma-ok assertion against a single-error Unwrap
interface alone never reaches past it. All three of providers.go's own
wrap sites use multi-%w (upstreamJSON's encode-failure path, "%w:
encode request body: %w"; upstreamBytes' build-request path, "%w: %w:
build request: %w"; and upstreamBytes' client.Do path, "%w: %w" — the
one a real request-context deadline reaches through). Result: a hung
upstream on the unified chat/embeddings/media routes recorded as 100%
healthy; only native passthrough (whose error is never re-wrapped by
upstreamBytes) worked.
Round 2's first attempt added a second hand-rolled interface
(multiUnwrapper, Unwrap() []error) and recursed into it — verified
correct against every wrap shape under `go test`, INCLUDING the exact
double-/triple-%w shapes providers.go itself produces. It still did not
work. tools/yaegi-check's own SHOULD-A extension (below) caught it: a
type assertion of a COMPILED concrete value (*fmt.wrapErrors, built by
the real fmt package) against an INTERPRETED interface type
(multiUnwrapper, declared in this plugin's own source) silently returns
ok=false under Yaegi, even though %T correctly reports *fmt.wrapErrors
and that type genuinely implements Unwrap() []error in real Go — the
reverse direction of the already-documented errors.As-on-interpreted-
types trap, not previously known to this codebase.
Fix: matchesSentinel now calls real errors.Is instead of hand-rolling
anything. This is safe here specifically, despite the "avoid errors.Is/
As" guidance elsewhere in this codebase: the panic risk that guidance
targets is errors.As checking whether an INTERPRETED type implements
error, which never arises for a plain sentinel comparison against
compiled-origin values (context.DeadlineExceeded, context.Canceled, and
every err this function ever sees) — the identical safe shape retry.go's
isTransient already relies on for these same two sentinels, proven
correct under Yaegi across every prior round. Empirically verified: this
is what made the extended yaegi-check harness pass; the hand-rolled
version did not, despite passing every compiled `go test` row.
Also in this commit:
- Route-level regression tests (TestHandleChat_
ContextDeadlineExceeded_RecordsProviderFailure, routes_unified_test.go;
TestHandlePassthrough_ContextDeadlineExceeded_RecordsProviderFailure,
routes_passthrough_test.go) — a real httptest upstream sleeping past a
real short request-context deadline, adapted from a reviewer-provided
probe. These are what exposed the blocker in the first place; the
sentinel-level table test alone (hand-built error shapes) is exactly
what let it ship.
- Prod-shape rows (double-/triple-%w, byte-for-byte providers.go's own
format strings) folded into the existing classification table
(renamed TestIsDeadlineExceeded_Classification — its own doc comment
now records that this table alone is NOT proof under the interpreter,
after round 2's own history).
- tools/yaegi-check/main.go (SHOULD-A): the harness's claim to prove the
interpreted deadline path was never actually true — its only upstream
answered instantly, so isDeadlineExceeded was never exercised with a
non-nil error at all. Added a second, deliberately slow provider
("slow", provider-prefixed model id so it cannot collide with the
existing fast one) driven with a short request-context deadline,
asserting failuresDay==1 under the real interpreter. This is the
harness that caught the blocker above.
- FOLDED-1: recordProviderAttempt's fire-and-forget store write
(SHOULD-5, round 1) is now bounded — limiter.spawnTokens, a per-limiter
buffered-channel semaphore (providerAttemptSpawnCap=64) the production
spawn closure acquires non-blockingly; a write that cannot start
immediately is DROPPED, not queued. Bounded and lossy-under-pressure by
design: telemetry only, enforcement never goes through spawn.
Per-limiter, not package-level, so the test suite's many parallel
*limiter instances cannot starve each other.
- FOLDED-2: TestRecordProviderAttempt_ProductionSpawn_
WriteEventuallyLandsAsync leaves limiter.spawn at its real production
default (every other test in this file overrides it to run
synchronously) and polls (waitUntil, deadline-bounded, not a fixed
sleep) for the counter to land — closing the -race async-path gap the
synchronous overrides otherwise leave entirely untested.
|
||
|
lukaszraczylo
|
1661cd5144 |
chore(admin): regenerate baked admin assets
Regenerate admin_assets_gen.go (webui/generate.mjs, gofmt -w) from the webui/ build produced by the preceding review-round webui commit, so admin.go's serveAdminPage/serveAdminAsset ship the MUST-1 contrast fix, day-window per-model badges, the accordion "Last refresh" fix, and the badge aria-labels. |
||
|
lukaszraczylo
|
9d079f093c |
docs(readme): provider success-rate counters and admin fields
SHOULD-3 (v0.22 review round). Documents: - The new llmgw:prov:*/llmgw:provmodel:* counter key families under Limits and accounting: format, minute/day-only windows and their TTLs, and the SHOULD-1 semantics — a timed-out attempt counts as a failure, a client-canceled one does not, both on top of retry.go's own transient classification, reused not forked. - Native passthrough's provider-level-only accounting limitation (no resolved model at attempt time). - The new adminProviderView fields (discoveryEnabled, attemptsDay/ failuresDay/attemptsMinute/failuresMinute, modelRates) under Admin's GET /admin/api/overview description. |
||
|
lukaszraczylo
|
abbf5d0967 |
fix(webui): accessible warn token, day-window model badges, review minors
Review round (v0.22) — MUST-1, SHOULD-2's frontend half, and every folded minor that touched webui/. MUST-1 (contrast): the degraded badge's amber previously reused --color-chart-tokens-out, tuned for a chart mark, not body text — measured ~2.22:1 against --card in light mode, a WCAG 1.4.3 failure on exactly the "something's wrong" tier. A new semantic --status-warn token pair (main.css, with a @theme inline mapping) replaces it: #a35c00 light (~5.06:1), #f0ac1f dark (~8.97:1, already passing, redefined anyway so every semantic-color token's dark value lives in one place). ProviderRateBadge.vue's degraded tier now uses border-status-warn/40 text-status-warn. SHOULD-2 (frontend half — see the backend commit for admin.go/limits.go): AdminModelRateView drops attemptsMinute/failuresMinute (the backend no longer sends them at all). ProviderRateBadge.vue's attempts-minute/ failures-minute props become optional; their absence (not a passed- through 0) triggers a new dayRateTitle fallback (lib/provider-rate.ts) — a real "0 attempts this minute" and "no minute data fetched at all" read identically as 0 otherwise, which would misreport a model badge as having live-minute data it never measured. Folded minors: - formatRatePercent floors instead of rounds — only a true 100% rate (zero failures) ever renders "100%"; 99.6% now shows "99%", not a round-tripped "100%" that would overstate it. - providerSuccessRate NaN-guards both inputs (Number.isFinite). - ProvidersView's modelRateFor uses `p.modelRates?.[model]` — a stale client against an older server build could otherwise throw rather than fall back, matching its own doc comment's stated intent. - ProviderRateBadge's title is now also its aria-label — title alone is mouse-only; a keyboard/screen-reader user previously got none of the minute/day detail. - The accordion's "Last refresh" DETAIL row (not just the trigger's compact text) now shares refreshLabel's three-state logic via a new refreshDetailLabel (lib/format.ts) — it previously called formatTimestamp directly, so a discovery-off provider's expanded row still said "never" beside a trigger that correctly said "discovery off". - providerModelScopeID's Go doc comment (limits.go, backend commit) cross-references configNamePattern for why the "/" join is collision- safe even though a model id may itself contain "/". - useTabHash.ts documents its two intentional URL-normalization quirks: case-sensitive hash matching, and an invalid/absent initial hash normalizing only in memory, not in the address bar, until the next tab change. |
||
|
lukaszraczylo
|
56817b41d1 |
test(yaegi): prove attempt-accounting under the interpreter
SHOULD-4 (v0.22 review round): tools/yaegi-check now drives a real chat completion against a local httptest upstream, then GET /admin/api/overview, asserting the resulting provider reports exactly one attempt and zero failures — end to end under Yaegi, not merely compiled: runUnified's attemptRecorder -> retryPolicy.do's context lookup -> limiter.recordProviderAttempt's isTransient/isDeadlineExceeded classification -> its fire-and-forget spawn -> buildAdminOverview's own batched providerUsage read. This also proves the SHOULD-1 hand-rolled errors.Unwrap walk (matchesSentinel/isDeadlineExceeded, limits.go) runs correctly interpreted, which `go test` cannot: that class of bug is exactly what this harness exists to catch. Adapted from a reviewer-provided reference harness (a second json.Unmarshal layering a mock-upstream provider and an admin-flagged user on top of the plugin's own .traefik.yml testData) rather than reinvented — reworked to poll GET /admin/api/overview briefly (SHOULD-5's fire-and-forget write can lag a single immediate read) and to assert on the decoded attemptsDay/failuresDay values instead of only printing them, so a regression here actually fails the gate. |
||
|
lukaszraczylo
|
1b4516efbb |
fix(providers): deadline failures, cheaper polling, async writes
Review round (v0.22): 1 must-fix + 5 should-fix. This covers the three backend items. MUST-1's WCAG fix lives in the webui commit; this is the three provider/ admin-side SHOULD items. SHOULD-1 (timeout blindness): a hung upstream previously read as 100% healthy — isTransient (retry.go) deliberately excludes context.DeadlineExceeded from what it retries (retrying past the gateway's own deadline is pointless), and recordProviderAttempt reused that same classification unforked, so a timed-out attempt recorded no failure. Fixed narrowly in recordProviderAttempt only, per the ruling: isTransient itself stays untouched and shared. A new isDeadlineExceeded adds exactly one extra condition — a context.Canceled error (the client walked away) still counts as an attempt only, from both functions alike. The sentinel match (matchesSentinel) is a hand-rolled errors.Unwrap walk, not errors.Is: providerHTTPError's own doc comment already documents that errors.As panics under Yaegi checking whether an interpreted pointer type implements error; errors.Is walks the identical internal machinery and carries the same risk, so this sidesteps it with a bare `err.(sentinelUnwrapper)` comma-ok assertion instead — plain interface satisfaction, not reflection over a possibly-interpreted type. SHOULD-2 (poll fan-out): a (provider, model) scope's minute-window counters fed nothing but a badge's title text on the Providers tab, so a dashboard with N configured models paid N wasted minute-window reads on every 5s poll. providerCounterKeys/providerUsage now read a kindProviderModel scope's day window only (2 keys, down from 4); a kindProvider scope is unaffected (still 4 — one scope per provider, not per model, so the read amplification never applied there). adminModelRateView drops AttemptsMinute/FailuresMinute entirely, not just unpopulated fields — the webui side (separate commit) falls back to a day-window-only title for a per-model badge. SHOULD-5 (TTFB): recordProviderAttempt's store write is telemetry-only — nothing in the request path gates on it — so it now runs via a new limiter.spawn dependency-injection field (mirrors nowFn/waitFn's existing pattern), defaulting to a real goroutine in production and overridden to run synchronously in every test that asserts on the resulting counter immediately afterward. This removes the synchronous Redis round trip this counter family added to the streaming TTFB path entirely. Loss on process shutdown is accepted (telemetry only). |
||
|
lukaszraczylo
|
080daebb53 |
chore(admin): regenerate baked admin assets
Regenerate admin_assets_gen.go (webui/generate.mjs, gofmt -w) from the webui/ build produced by the preceding feat(webui) commit, so admin.go's serveAdminPage/serveAdminAsset ship the Providers-tab rate badges, refresh labels, and anchor-link syncing. |
||
|
lukaszraczylo
|
2298249df5 |
feat(webui): rate badges, refresh labels, anchor links
Providers tab (Feature A): a new ProviderRateBadge component renders each provider's day-window success rate (attempts-failures)/attempts — always shown when attempts>0 (100% muted, <99% amber via the chart palette's --color-chart-tokens-out token, <90% destructive, reusing the Badge component's existing secondary/outline/destructive idioms rather than inventing a new variant), a distinct "no traffic" badge when attempts=0, and the live minute-window count in its title attribute. The same component renders beside a ModelChip inside a provider's accordion, but only when that model is degraded (<99% with attempts>0) so a long, healthy model list stays uncluttered. The math and its threshold constants live in lib/provider-rate.ts, unit-tested directly (vitest's node environment needs no component mount). Feature B: a provider's "refreshed ..." label (lib/format.ts's new refreshLabel) now reads "discovery off" when DiscoveryEnabled is false, "pending" when it is true but lastRefresh is still unset, and otherwise the unchanged relative-time text — replacing the misleading "refreshed never" a discovery-disabled provider showed before. Feature C: composables/useTabHash syncs App.vue's active tab to the URL hash (#providers/#usage/#charts/#targets, matching the existing TabsTrigger values) with no router dependency. On load it reads the current hash (falling back to "providers" for an absent or invalid one); on tab change it writes the hash back via history.replaceState (no scroll jump, no new history entry per click); a hashchange listener handles back/forward and hand-edited URLs. hashToTab/tabToHash are exported as pure functions so the mapping is unit-tested directly, independent of window/history/location — every DOM touch inside useTabHash itself is guarded behind one hasWindow check so the module still imports cleanly under vitest's node test environment. AuthGate never reads or writes the hash, so a tab named in the URL survives the unauthenticated -> authenticated transition unchanged. |
||
|
lukaszraczylo
|
bf23b41568 |
feat(admin): rates + discovery flag in API
Extend GET /admin/api/overview's existing providers payload rather than adding a sibling endpoint: the route already returns one providers array per request on the dashboard's 5s poll, is already admin-gated and stats-excluded, and the new fields are one more batched limiter read sharing that same request — a second endpoint would duplicate all three properties for no benefit. adminProviderView gains AttemptsDay/FailuresDay/AttemptsMinute/ FailuresMinute (the provider's own current-window counters) and ModelRates (a per-model breakdown, keyed by upstream model id, via the new adminModelRateView) — Feature A. buildAdminOverview builds every provider's and every (provider, model) pair's limitScope in two passes and reads them all through limiter.providerUsage in ONE storeGetMulti call, then slices the flat result back by index. adminProviderView also gains DiscoveryEnabled, mirroring a new providerSnapshot.discoveryEnabled field (registry.go) read straight off providerState — it is set once at construction and never mutated afterward, so it needs neither providerState's mutex nor a place in its existing snapshot() method's mutable-field return shape. Feature B: this lets the webui tell "discovery is off" apart from "discovery is on but has not refreshed yet", which previously both showed the same zero lastRefresh as an identical "refreshed never". |
||
|
lukaszraczylo
|
aafd2a0433 |
feat(providers): success-rate accounting
Track per-provider and per-(provider,upstream-model) upstream-attempt outcomes using the existing fixed-window counter machinery (limits.go), following mcp_a2a.go's per-target counter pattern: kind-embedded keys (kindProvider="prov", kindProviderModel="provmodel") keep a provider scope collision-safe against a user/group/MCP-target/agent of the same name. limiter.recordProviderAttempt(provider, model, resp, err) increments an "attempts" counter on every call, and a "failures" counter only for a provider-fault outcome — reusing retry.go's own isTransient(resp, err) classification exactly (transport/connection error, HTTP 429, or a 5xx), not forking it. Both counters are written at minute and day granularity only; provider health is a now-and-today question, not a billing one, so there is no month window. limiter.providerUsage batches every scope's read into one storeGetMulti round trip, mirroring targetUsage's own single-batch discipline. Wired at every chokepoint an upstream HTTP attempt resolves: - providers.go's upstreamJSON/upstreamRawBytes path (chat, embeddings, images, audio) via retry.go's retryPolicy.do, which now looks up an optional attemptRecorder from ctx and invokes it once per retry attempt (attempt-accounting: a request retried twice before succeeding reports three attempts, not one). - routes_unified.go's runUnified (chat/embeddings, streaming and non-streaming alike — both funnel through the same upstreamJSON call). - routes_media.go's three media handlers (images.generations, audio.speech, audio.transcriptions). - routes_passthrough.go's proxyUpstream (native provider passthrough) — a single un-retried attempt, provider-level only: the upstream model lives in the response body, read only after the attempt resolves. The attemptRecorder is threaded via context (providers.go's withAttemptRecorder/attemptRecorderFromContext), not a new function parameter on every call site — proxyUpstream and retryPolicy.do are both shared with paths that must NOT record provider accounting (registry.go's discovery listModels calls, and the MCP/A2A target proxy, mcp_a2a.go's handleTargetProxy), so scoping via context keeps their signatures unchanged and those callers correctly opt out by simply never wrapping their context. |
||
|
lukaszraczylo
|
ad5adbcc1d |
docs(readme): handshake fallback, legacy-SSE exclusion, protocol allowlist (SF5)
Rewrote the federated POST /mcp bullets to document the review round 2 fix batch: per-backend timeouts (20s tools/list, 120s tools/call), the bare-first-with-fallback session handshake and exactly what triggers it (JSON-RPC error or HTTP 4xx, never a network failure or 5xx), the legacy HTTP+SSE-transport exclusion (URL path ending '/sse') and why no handshake can fix it, the loud 'no MCP server reachable' error when every attempted server fails, non-2xx/ empty-envelope rejection, the protocol-version allowlist, and the new 405 for a disallowed method on /mcp. Extended the admin-API GET /admin/api/targets bullet, the total-scope accounting list, and the rejected-before-quota bullet to include the federated endpoint. Architecture diagram gains a Federated branch node (render-checked with mmdc). |
||
|
lukaszraczylo
|
12d55e66ef |
test(integration): mocktool JSON-RPC + federated /mcp coverage (SF4)
mocktool's 'tool' mode gains two federated-/mcp JSON-RPC endpoints:
'/' (lenient — every bare, session-less call succeeds, matching 8 of
11 real servers the live probe found) and '/strict' (session-required
— rejects a bare tools/list/tools/call with a JSON-RPC error until a
prior 'initialize' call's Mcp-Session-Id header accompanies it,
matching readitall/fetch's real behavior). A new 'toolstrict'
MCPServers entry in dynamic.yml.tmpl points at '/strict'.
TestFederatedMCP drives POST /mcp against real Traefik+Yaegi: a
federated tools/list must include both tool_echo (bare path) and
toolstrict_echo (reachable only via the plugin's own
bare-first-with-fallback handshake retry, mcp_federation.go's
mcpBackendCall), and a tools/call to toolstrict_echo must route
correctly with the prefix stripped. Closes the review gap ('only
goroutine in the package, zero real-interpreter coverage') the
go-test-only suite left: the handshake fallback, and the plugin's
goroutine fan-out in mcpFederatedToolsList, now both run for real
under Yaegi, not just against go test's in-process mocks.
Verified: make integration — ok github.com/lukaszraczylo/
traefik-llmgateway/integration 40.113s, exit 0, no FAIL/panic/skip in
the full run.
|
||
|
lukaszraczylo
|
d5a5c3e064 |
chore(admin): regenerate baked admin assets
Regenerated via webui/generate.mjs from a fresh vite build after the resilient-dashboard-polling fix (SF7, previous commit). |
||
|
lukaszraczylo
|
a25ebf0567 |
fix(webui): resilient dashboard polling (SF7)
useDashboardStore.refresh() used Promise.all across overview/usage/ targets: one route rejecting threw before any of the three state fields were assigned, so a single flaky GET /admin/api/targets blanked the already-working Providers and Usage views too. Switched to Promise.allSettled with per-section assignment: a fulfilled result updates its own field, a rejected one leaves the PREVIOUS value in place rather than nulling it. lastUpdated still advances on a partial success (some fresh data did arrive); error names which section(s) failed. A 401/403 on any section still defers to the auth store exactly as before (AdminApiError check unchanged). Added dashboard.spec.ts — the first Pinia store under vitest in this package (a store needs no DOM, so it fits the existing 'node' environment without pulling in jsdom); vite.config.ts's own test-scope comment updated to say so. |
||
|
lukaszraczylo
|
8afb903227 |
fix(mcp): backend timeouts, response validity, session handshake fallback
Review round 2 fix batch for the federated /mcp endpoint, driven by a
live probe against all 11 production MCP servers behind this gateway
through the byte-pipe proxy (session-less tools/list): 8 answer bare,
readitall and fetch reject it and need a session handshake, playwright
is a legacy HTTP+SSE-transport server (400 Missing sessionId) that no
handshake fixes.
MF1: every backend call now runs under its own context.WithTimeout
derived from r.Context() — toolsListBackendTimeout (20s) per fan-out
call, toolsCallBackendTimeout (120s) for tools/call, named constants.
MF2: doBackendJSONRPC rejects any non-2xx upstream status as an error
(previously unchecked — a non-JSON-RPC error body only failed by
accident of not parsing). parseBackendJSONRPC additionally rejects a
validly-JSON body that carries neither "result" nor "error" — the
exact silent-failure shape that would otherwise decode into an
all-zero jsonrpcResponse and hand the client a bare
{"jsonrpc":"2.0","id":1}. mcpFederatedToolsCall carries a
defensive final guard against the same shape before relaying.
MF3: mcpBackendCall is now bare-first-with-fallback — attempt the
bare call; on a JSON-RPC error OR HTTP 4xx (never a network failure,
timeout, or 5xx), retry once via initialize -> capture Mcp-Session-Id
-> retry with the header -> best-effort DELETE close. A legacy
HTTP+SSE-transport server (URL path ending "/sse") is excluded from
federation entirely before ever reaching this — isLegacySSETransportURL,
folded into allowedMCPServerNames — since no handshake fixes a
different transport; it stays reachable via the per-server proxy. When
every attempted server fails, tools/list returns a JSON-RPC internal
error ("no MCP server reachable") instead of a silently-empty list.
SF6: defaultMCPProtocolVersion -> "2025-11-25"; a caller's own
protocolVersion is echoed back only from a small explicit allowlist
(knownMCPProtocolVersions), falling back to the default otherwise —
never echoes an arbitrary caller string as this gateway's own claimed
capability.
SF8: GET/DELETE/etc. on exactly /mcp is now 405 with Allow: POST,
checked before auth (llmgateway.go).
Folded minors: countTargetRequests batches per-target counter writes
for tools/list's whole fan-out into ONE storeIncrMulti call after
wg.Wait() (was one call per server); every "actually contacted" doc
comment now says "attempted" (attribution is attempt-, not
success-, accounting); lastSSEDataLine documents its no-cross-message-
id-correlation limitation; routes_unified.go's stale buildLimitScopes/
withTotalScope comments (which claimed handleAdminAPI calls
buildLimitScopes directly — it never has) are corrected; jsonrpcRequest
documents why its "jsonrpc" field is decoded but never validated.
Tests: ~30 new/updated cases covering per-backend timeout derivation
(via the shorter-of-two-context-deadlines technique, no real 20s/120s
waits), non-2xx rejection, the empty-envelope guard end-to-end,
handshake-trigger classification (table-driven: JSON-RPC error and
HTTP 4xx trigger, HTTP 500 and network failure do not), the full
handshake sequence (bare -> initialize -> retry -> close, asserted
call-by-call), handshake-gives-up-too, all-servers-failed, legacy-SSE
exclusion (never contacted, in both tools/list and tools/call),
protocol-version allowlist (all 4 versions + one rejected), 405/Allow,
and best-effort session-close failure never surfacing to the caller.
|
||
|
lukaszraczylo
|
d565c1d4ea |
feat(mcp): federated /mcp aggregation endpoint
New POST /mcp: a single JSON-RPC 2.0 endpoint aggregating every MCP
server the caller's group can reach, for clients built against the
earlier pre-plugin gateway's one aggregated endpoint (pugbot's
mcpclient, agentkit) rather than this plugin's per-server
/mcp/{name}/... proxy.
Unlike the per-server proxy, this cannot be a byte-level reverse
proxy: aggregating N servers' tools/list results, or routing one
tools/call to the single right server, both require actually parsing
the JSON-RPC envelope. The gateway answers initialize/ping locally
and, for tools/list/tools/call, issues its own outbound JSON-RPC
call(s) to the relevant backend server(s) per incoming request — no
session state persisted across separate client requests, matching the
per-server proxy's own complete absence of gateway-side session
tracking. A backend may answer bare JSON or an SSE stream; both are
accepted (parseBackendJSONRPC).
- tools/list fans out concurrently to every allowed server and merges
results, prefixing each tool "<serverName>_<toolName>" — the
underscore convention the earlier agentgateway used and that
pugbot/agentkit already persist in their own tool-id records (e.g.
brave-search_brave_web_search, verified live). A failing/unreachable
server is skipped, not a whole-call failure.
- tools/call resolves the target by LONGEST matching
"<serverName>_" prefix against the caller's allowed servers
(disambiguates a server name that prefixes another), strips it,
forwards, and relays the result/error under the CLIENT's own id. An
unresolvable prefix (unconfigured, or authz-excluded) is a JSON-RPC
invalid-params error, not a 404.
- Reuses Feature B's countTargetRequest for per-target attribution —
every backend server actually contacted increments its own scope.
- notifications/* -> 202 no body; unknown method -> JSON-RPC
method-not-found; malformed body -> JSON-RPC parse error with
id:null (spec-required, not omitted).
No new limits config surface. Tests cover prefix resolution
(table-driven, incl. the ambiguous-prefix case), SSE/JSON backend
parsing, group-restricted aggregation (a denied server is never even
contacted), call routing/prefix-stripping, backend error relay,
unreachable-server handling, unknown-prefix errors, and per-target
counter attribution. README's MCP and A2A section documents the
federation contract and naming convention; the architecture diagram
and the total-scope/rejected-before-quota notes are updated to
include the new route.
|
||
|
lukaszraczylo
|
94d8f90805 |
chore(admin): regenerate baked admin assets
Regenerated via webui/generate.mjs from a fresh vite build after the MCP & Agents tab (feat(webui) commit above). |
||
|
lukaszraczylo
|
7ad573b63e |
feat(webui): MCP & Agents admin tab
New 'MCP & Agents' tab (after Charts) fed by GET /admin/api/targets: two Card panels, each a sortable DataTable (reusing DataTable/ SortHeaderButton — no hand-rolled table markup) listing configured MCP servers and agents with name, url (muted, truncated with a title tooltip), access (Badge per allowed group, muted 'All groups' when unrestricted — the same idiom UsageView's own group Access block already uses), and req/min, req/day, req/month. - types/api.ts: AdminTargetCountersView/AdminTargetView/ AdminTargetsResponse matching admin.go's JSON exactly. - stores/dashboard.ts: targets fetched in the SAME Promise.all/5s poll as overview and usage — one store, one timer, not a third. - lib/target-columns.ts: shared ColumnDef set both DataTables use. - components/TargetsView.vue: SearchInput + useSearchQuery name filter (inline matching, mirroring ProvidersView's own single-field aliasMatches — not every filter needs a lib/*-search.ts module). |
||
|
lukaszraczylo
|
66fa0b56ea |
feat(admin): MCP/agent target visibility
Add per-target request accounting for the MCP/A2A proxy path and a
new admin endpoint to surface it.
- limits.go: countTargetRequest increments one target's req counters
at min/hour/day/month in a single storeIncrMulti call — pure
accounting, no limits, mirroring account()'s own contract.
targetUsage batches the read side the same way currentUsage does
for user/group scopes.
- mcp_a2a.go: handleTargetProxy attributes every admitted proxied
request to its target's own scope, kind "mcp" or "agent"
(scopeKindAgent — deliberately not targetKindAgent's "a2a" routing
convention). kind is embedded in the counter key (windowKey), so a
target can never collide with a user/group/total scope of the same
id, whatever any of them are named.
- admin.go: new GET /admin/api/targets, admin-gated and
stats-excluded exactly like the other three /admin/api/* routes.
Access lists are computed via the SAME matchesGlob call
group.allowsMCP/allowsAgent themselves use, so the panel can never
disagree with enforcement; a target every group can reach collapses
to an omitted access list ("empty meaning all").
No new config surface: target scopes carry no configurable limits
this round.
Tests: TestTargetScopeKind (mapping), TestHandleTargetProxy_
PerTargetCounters_AttributedAndCollisionSafe (table-driven over
mcp/a2a, proves counters land on the right scope and never collide
with a same-named user), TestAdminTargets_* (gating tables extended,
shape/sorting/URL-sanitization/access-list/counter assertions).
|
||
|
lukaszraczylo
|
1183b6fa3c |
feat(limits): unconditional usage accounting
buildLimitScopes previously omitted a user or group scope entirely whenever that entity had no configured Limits, so a limit-less user/group never accumulated ANY usage counters. Root-caused live: the admin dashboard's total kept climbing while every per-user row stayed zero until an operator added phantom, unreachable requestsPerDay limits everywhere just to make counters build at all. buildLimitScopes now always builds a user and a group scope. Limits still gate enforcement only: evaluateScope already no-ops on a nil Limits field, so a limit-less scope is counted but never violates. Update stale doc comments coupling accounting to configured limits (evaluateScope, the integration alice fixture, routes_media_test.go, routes_unified_test.go, admin_test.go's admin1 assertion) and add: - TestBuildLimitScopes_AlwaysBuildsBothScopes: table-driven over every nil/non-nil user/group limits combination. - TestAdminUsage_LimitlessUserAndGroup_AccumulateRealTraffic: end-to-end regression proving a limit-less user and group accumulate real request/token counters visible via GET /admin/api/usage. |
||
|
lukaszraczylo
|
90545df588 | docs: drop reference to local design-doc path | ||
|
lukaszraczylo
|
0530a4a2ff | chore: ignore local docs/superpowers workspace | ||
|
lukaszraczylo
|
87faa9e096 | chore(lint): suppress gosec G704 on operator-configured redis dial | ||
|
lukaszraczylo
|
1c511085d8 | chore(admin): regenerate baked admin assets | ||
|
lukaszraczylo
|
b081055aaa |
fix(admin): wire-level access tests, MCP/agent UI, glob-safe chips
Review fix round on the group-access-display feature (Should-Fix + folded items): 1. TestAdminUsage_GroupAccessLists now asserts at the raw-JSON level, not just the typed adminUsageResponse struct: decodes rows into map[string]json.RawMessage and checks the four access-list keys are ENTIRELY ABSENT on user/total rows and an unrestricted group, PRESENT with exact values on a restricted group. A typed-struct decode alone cannot prove omitempty is doing its job -- a present-but-null field decodes into the same nil []string a genuinely absent key would. Verified by temporarily stripping the omitempty tags: the new assertions fail (four keys read back 'null'); the old struct-only assertions stayed green throughout, confirming the gap the review flagged. 2. UsageView.vue's Access block now renders MCP servers and Agents too, same Badge treatment as Providers, with muted 'All MCP servers'/ 'All agents' fallbacks -- GroupConfig carries all four lists (llmgateway.go), so the display now matches the payload instead of dropping two of the four fields silently. 3. A configured Models entry containing *, ?, or [ (path.Match's own special characters -- auth.go's matchesGlob) is a pattern, not a routable id: it now renders as a plain, non-copyable Badge (font-mono, secondary) instead of the copyable ModelChip, so a caller can no longer copy a glob pattern into a request's model field thinking it's a real id. Every Access label (Providers/Models/MCP servers/Agents) also gets a title attribute (one shared GLOB_HINT constant) making the glob-match semantics discoverable, since all four lists are glob-matched, not just Models. 4. groupSummary's providers/models/mcpServers/agents are now independent copies (cloneStringSlice), not aliases of the live group.* slices authorization reads on every request -- structurally prevents a future display-side sort/mutation from ever reordering or corrupting enforcement data. slices.Clone was tried first and reverted: it broke make yaegi-check (yaegi's stdlib symbol table does not resolve the generic slices package's type parameters under interpretation), so cloneStringSlice uses plain make/copy builtins instead -- verified yaegi-check passes with this version. 5. README.md's usage endpoint section: fixed the stale 'Overview and Usage views' reference (now 'Providers', matching the earlier tab rename) and added one line naming the new providers/models/ mcpServers/agents group-access fields, matching the section's existing terseness. 6. Renamed the 'unrestricted group carries empty/omitted lists' subtest to 'empty or omitted' -- the slash broke -run subtest path matching. 7. The Access block now uses the same dl/dt/dd markup as the sibling stat grid immediately below it, instead of a bespoke div/p structure -- one convention per panel. |
||
|
lukaszraczylo
|
d99b959ee6 | chore(admin): regenerate baked admin assets | ||
|
lukaszraczylo
|
03bc67a5f0 |
feat(admin): expose group provider/model access
GET /admin/api/usage's group rows now carry the group's configured Providers/Models/MCPServers/Agents (GroupConfig, llmgateway.go), echoed exactly as configured -- never resolved/expanded to the full catalog server-side. Empty/unset stays omitted (omitempty), matching GroupConfig's own JSON convention and group.allowsX's empty-means-all semantics (auth.go): the dashboard can tell 'restricted to these N' apart from 'unrestricted' without the payload growing with catalog size. groupSummary (auth.go) gains providers/models/mcpServers/agents, carried through from authStore.snapshot unchanged. adminUsageEntryView (admin.go) gains the matching JSON fields, populated only for group entries -- a user or the total scope row never carries them, mirroring GroupName's own user-only convention. The Usage view's group accordion (webui/UsageView.vue) renders an Access block above the existing stat grid: Badge chips for explicit providers, ModelChip for explicit models (both shadcn/existing components, no hand-rolled chips), or muted 'All providers'/'All models' text when unrestricted. Display-only this round -- not wired into the user/group search filter. types/api.ts's AdminUsageEntryView mirrors the new optional fields. Tests: admin_test.go's new table-driven TestAdminUsage_GroupAccessLists covers a restricted group's exact lists, an unrestricted group's empty/omitted lists, and that a user/total row never carries them. |
||
|
lukaszraczylo
|
bb4fa4420e |
refactor(webui): rename Overview tab to Providers
Mechanical rename: OverviewView.vue -> ProvidersView.vue (file, component import, tab label/value in App.vue), plus doc-comment/README references across webui/ that named the view by its old tab label. The backend GET /admin/api/overview endpoint, adminOverviewResponse type, and the frontend AdminOverviewResponse/dashboard.overview field all keep their original names unchanged (API surface) -- only the UI tab label and the component/file that renders it changed. No behavior or layout change to the view itself. |
||
|
lukaszraczylo
|
22006c5e25 | chore(admin): regenerate baked admin assets | ||
|
lukaszraczylo
|
5408b8e895 |
fix(webui): address auth-gate review findings
Explicit refocus. Add a watch(() => auth.error, ...) that calls the same focusKeyInput() helper onMounted already used. Previously, refocus-after-failure only worked through an undocumented chain (auth.submit() optimistically flips isAuthenticated, App.vue unmounts AuthGate, a 401 calls auth.reject(), isAuthenticated flips back, AuthGate remounts, onMounted refires). That chain still exists and still drives the shake retrigger; a comment above the Input now says so explicitly, so a future change to optimistic auth does not break either behavior silently. Security disclosure order. Move the three-item checklist above the form, so the claims sit in the reading path before the key field instead of below the submit button. Add a visually hidden `<h2 id="authgate-security-heading">` and tie the `<ul>` to it via aria-labelledby, giving screen-reader users a label the list did not have before. Wording and text-xs sizing are unchanged. Invalid-state wiring. Add :aria-invalid and :aria-describedby to the Input, pointing at a new id="authgate-error" on the Alert, closing the loop between the input and its own error message. Typography hierarchy. CardTitle grows to text-lg font-semibold; the "LLM Gateway" wordmark drops to a muted-foreground text-xs eyebrow, so the title now reads as the dominant line instead of losing to the wordmark's former text-sm font-semibold. Drop text-base from the Input's class list — Input.vue already sets text-base (overridden by its own md:text-sm at >=768px); the local copy did nothing but restate the default. h-10/pr-10 stay. |