feat(token-rate-limit): add shared Valkey quota enforcement - #790
feat(token-rate-limit): add shared Valkey quota enforcement#790nerdalert wants to merge 623 commits into
Conversation
…xy#236) Signed-off-by: Sébastien Han <seb@redhat.com>
…praxis-proxy#241) Signed-off-by: Sébastien Han <seb@redhat.com>
…xis-proxy#253) Signed-off-by: Alex Snaps <alex@wcgw.dev>
…praxis-proxy#244) Signed-off-by: Sébastien Han <seb@redhat.com>
…raxis-proxy#235) Signed-off-by: Sébastien Han <seb@redhat.com>
…proxy#234) Signed-off-by: Sébastien Han <seb@redhat.com>
Signed-off-by: Sébastien Han <seb@redhat.com>
Signed-off-by: Sébastien Han <seb@redhat.com>
…xis-proxy#254) Signed-off-by: Sébastien Han <seb@redhat.com>
…axis-proxy#242) Signed-off-by: Sébastien Han <seb@redhat.com>
…y#247) Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Signed-off-by: Aslak Knutsen <aslak@4fs.no>
Signed-off-by: Sébastien Han <seb@redhat.com>
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
…nse-store (praxis-proxy#271) Signed-off-by: Sébastien Han <seb@redhat.com>
Signed-off-by: Sébastien Han <seb@redhat.com>
…roxy#272) Signed-off-by: Sébastien Han <seb@redhat.com>
Signed-off-by: Sébastien Han <seb@redhat.com>
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> Co-authored-by: Francisco Javier Arceo <farceo@redhat.com> Co-authored-by: Sébastien Han <seb@redhat.com>
…raxis-proxy#274) Signed-off-by: Sébastien Han <seb@redhat.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com> Co-authored-by: Sébastien Han <seb@redhat.com>
…04) (praxis-proxy#290) Signed-off-by: Sébastien Han <seb@redhat.com>
…-proxy#270) Signed-off-by: Sébastien Han <seb@redhat.com>
…roxy#293) Wire the openai_stream_events filter into the Responses API full-flow pipeline between openai_response_store and openai_responses_rehydrate. Signed-off-by: Sébastien Han <seb@redhat.com>
…is-proxy#299) docs(filters): generate missing openai_stream_events doc and lint in CI Run `cargo xtask generate-filter-docs` to produce the missing `openai_stream_events.md` and refresh stale docs (a2a, mcp, openai_response_store, reference index). Add `cargo xtask lint-filter-docs` to the `make lint` target so CI catches stale or missing filter documentation going forward. Signed-off-by: Sébastien Han <seb@redhat.com>
This also removes CI shims to build and test with the main branch while we were waiting for this release. Signed-off-by: Shane Utt <shaneutt@linux.com>
…axis-proxy#308) The lint-filter-docs check fails in CI because parse_shared_config_items() hardcodes ../praxis to find shared config types. Since praxis moved from a path dependency to a crates.io dependency (v0.4.0), ../praxis does not exist in CI. Fall back to resolving praxis-proxy-filter source via cargo metadata when ../praxis is not available. Also trim unused core/tls source parsing since only filter/payload_processing types (OnInvalidBehavior) are referenced by AI filter configs. Signed-off-by: Sébastien Han <seb@redhat.com>
…dencies group (praxis-proxy#307) chore(deps): bump jsonwebtoken in the rust-dependencies group Bumps the rust-dependencies group with 1 update: [jsonwebtoken](https://github.com/Keats/jsonwebtoken). Updates `jsonwebtoken` from 9.3.1 to 10.4.0 - [Changelog](https://github.com/Keats/jsonwebtoken/blob/master/CHANGELOG.md) - [Commits](Keats/jsonwebtoken@v9.3.1...v10.4.0) --- updated-dependencies: - dependency-name: jsonwebtoken dependency-version: 10.4.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: rust-dependencies ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sébastien Han <seb@redhat.com>
Signed-off-by: Shane Utt <shaneutt@linux.com>
Signed-off-by: Shane Utt <shaneutt@linux.com>
Signed-off-by: Shane Utt <shaneutt@linux.com>
Signed-off-by: Shane Utt <shaneutt@linux.com>
…s-proxy#787) * fix(compat): align routing branches with terminal semantics Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> * fix(compat): support Praxis main retry context fields Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> * fix(compat): retain tracing guard Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> --------- Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
…xy#775) Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 9.0.0 to 10.0.1. - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@c771a70...20cfd1b) --- updated-dependencies: - dependency-name: astral-sh/setup-uv dependency-version: 10.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…axis-proxy#777) Bumps [github/codeql-action/init](https://github.com/github/codeql-action) from 4.37.6 to 4.37.7. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@5595cca...ff2f1c6) --- updated-dependencies: - dependency-name: github/codeql-action/init dependency-version: 4.37.7 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…raxis-proxy#776) Bumps [github/codeql-action/analyze](https://github.com/github/codeql-action) from 4.37.6 to 4.37.7. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@5595cca...ff2f1c6) --- updated-dependencies: - dependency-name: github/codeql-action/analyze dependency-version: 4.37.7 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Aslak Knutsen <aslak@4fs.no>
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review
Summary: Adds a token_rate_limit filter with in-memory and Valkey backends for authenticated, model-scoped token quota enforcement. Also adds producer-defined selection groups, a picker module for round-robin/random/deterministic selection, and pipeline ordering validation.
Overall: The design is well thought out and the in-memory ledger is solid. However, the Valkey Lua scripts have a key-scoping bug in the expiry cleanup path that will cause the active-tokens counter to inflate monotonically, eventually denying all requests. The on_response_body sync path also propagates reconciliation queue errors in a way that could disrupt response delivery.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 1 |
| Medium | 2 |
| redis.call('HINCRBY', physical .. ':settled', bucket, amount) | ||
| redis.call('ZADD', physical .. ':settled-index', bucket, bucket) | ||
| redis.call('HDEL', active_key, reservation) | ||
| redis.call('DECRBY', physical .. ':active-tokens', amount) |
There was a problem hiding this comment.
[Large] The expiry cleanup path constructs key names by appending :active-tokens and :settled-index to the per-key physical prefix, but the admission and reservation paths use the rule-scoped KEYS[9] ({rule_prefix}:active-tokens) and KEYS[8] ({rule_prefix}:settled-index). These are different Redis keys.
Consequences:
KEYS[9](rule-scopedactive-tokens) is incremented on every reservation (line 211) and decremented on normal reconciliation (reconcile script line 231), but never decremented during expiry cleanup. Every expired reservation permanently inflates this counter.- The settled tokens from expired reservations are written to a stray per-key
settled-indexthat is never queried during admission (line 198 readsKEYS[8]), so those tokens vanish from capacity accounting. - Over time,
active_sumgrows without bound, causingsettled_sum + active_sum + estimate > capacityto deny all requests even when there are no actual active reservations.
Change lines 158-161 to use the rule-scoped keys that admission actually reads:
redis.call('HINCRBY', settled, bucket, amount)
redis.call('ZADD', settled_index, bucket, bucket)
redis.call('HDEL', active_key, reservation)
redis.call('DECRBY', active_tokens_key, amount)settled (line 136) is KEYS[2] which is per-key -- that is correct for the settled hash. But settled_index and active_tokens_key must be the rule-scoped variables assigned at the top of the script (lines 138-139), not constructed from physical.
| estimate: rule.estimate, | ||
| now_ms: self.now_ms(), | ||
| }) | ||
| .map_err(|error| -> FilterError { error.into() })?; |
There was a problem hiding this comment.
[Medium] on_response_body is a sync callback that propagates enqueue_reconcile errors via ?, returning a FilterError. If the Valkey reconciliation channel is full (bounded at 1024), this fails the response body path for a request that was already admitted and served. Reconciliation is a post-response accounting concern; its failure should not disrupt the response being delivered to the client.
Change this to log the error and continue rather than propagating it:
if let Err(error) = rule.backend.enqueue_reconcile(ReconcileRequest { ... }) {
tracing::error!(%error, "token-rate-limit: failed to enqueue reconciliation");
counter!("praxis_ai_token_rate_limit_backend_errors_total", "backend" => "valkey", "operation" => "enqueue_reconcile").increment(1);
}| return Err(format!("listener '{}': token_count must follow token_rate_limit", listener.name).into()); | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
[Medium] validate_token_rate_limit_order has no unit tests. The existing test module covers provider boundary validation, misaligned clusters, and open security filters, but nothing exercises the three code paths here: (1) token_rate_limit after intelligent_route is rejected, (2) token_count before token_rate_limit is rejected, (3) correct ordering is accepted. Add tests paralleling the provider boundary tests to cover these constraints.
…y#800) * test: opt loopback fixtures into allow_private_endpoints praxis-core main now validates clusters defined inline in load_balancer filters and rejects private/loopback/link-local endpoints unless the config sets `insecure_options.allow_private_endpoints: true` (core commit 894ac18, "validate clusters defined inline in load-balancer filters"). Core updated its own conformance fixtures and shipped examples for this change, but nothing propagated downstream, so the whole `ai` tree fails `test-praxis-main` (unit, schema, and integration steps) against core main. The flag already ships in released praxis 0.5.3, so this fix is release-independent and green on both the 0.5.3 pin and core main. Mirror core's remediation: - Append a top-level `insecure_options.allow_private_endpoints: true` block to every shipped example config whose inline load_balancer cluster targets a loopback/private backend (52 files). - Port core's `allow_loopback_endpoints()` test helper into `praxis-test-utils` so harness-loaded example configs opt in centrally at load time. - Add the flag inline to every raw-parse fixture (schema, integration, and the `dump`/`pipelines` unit tests) that builds an inline load_balancer cluster on a private endpoint. Public-endpoint fixtures, listener addresses, ip_acl allow lists, tcp upstreams, and top-level `clusters:` blocks are left untouched. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * test(inference): merge insecure_options in replay collision tests Two replay negative tests build their config from the shipped `responses-proxy.yaml` example and then append their own top-level `insecure_options:` block (`allow_public_admin` / `allow_private_health_checks`). Now that the example carries its own `insecure_options:` opt-in, string concatenation produced a duplicate top-level key, changing the parse outcome so the intended rejection error no longer fired. Merge each test's option under the example's existing `insecure_options:` block instead of emitting a second one, restoring the asserted errors. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> --------- Signed-off-by: usize <mofoster@redhat.com>
9a9ecf4 to
ad4ccf7
Compare
praxis-proxy#721) * fix(guardrails): fail closed on NeMo redact verdict until body replacement is ready GuardResult::Redact was returning FilterAction::Continue while recording status=redacted, silently forwarding the original unmodified body. This leaked content the configured guardrail identified as sensitive and misled downstream branch logic into treating the request as sanitised. Change record_verdict to return FilterAction::Reject(403) for GuardResult::Redact, identical to the GuardResult::Block path, so the original body is never forwarded. Body replacement with the provider's modified_text is deferred to praxis-proxy#579. Update on_request_body_modified_writes_filter_results to assert the 403 rejection and rename it accordingly. Add regression test on_request_body_modified_never_forwards_original_secret that asserts the action is Reject, the original SSN is absent from the body buffer, and status is never recorded as passed. Fixes: fnd_sig-feat-custom-ai-anthropic-gua_cfe2e666bb Signed-off-by: mkoushni <mkoushni@redhat.com> * test(guardrails): assert original secret absent from rejection body Replace the weaker matches!(action, Reject(_)) assertion in on_request_body_modified_never_forwards_original_secret with a direct check that the raw SSN never appears in the rejection body. This makes the test meaningfully different from on_request_body_modified_rejects_with_403 and directly validates the stated invariant: the unmodified user content must not surface in any response path. Reported by praxis-bot review of fix/guardrails-redact-fail-closed. Signed-off-by: mkoushni <mkoushni@redhat.com> * fix(guardrails): extract nemo_pii_redact_filter helper to fix too-many-lines lint Both on_request_body_modified_rejects_with_403 and on_request_body_modified_never_forwards_original_secret share identical mock setup. Extract it into nemo_pii_redact_filter() to bring each test under the 30-line clippy limit. Also collapse the two-line rejection body setup into one expression using rejection.body.as_deref().unwrap_or_default(). Signed-off-by: mkoushni <mkoushni@redhat.com> * fix(guardrails): format assert chain to satisfy rustfmt Collapse the split method chain onto a single line as required by cargo +nightly fmt --check. Signed-off-by: mkoushni <mkoushni@redhat.com> * fix(guardrails): drop stale praxis-proxy#579 references from redact-fail-closed path Issue praxis-proxy#579 is closed. Remove all references to it from the comment, the warn! log message, the unit-test assertion, and the integration-test doc comment and assertion. The fail-closed behaviour for GuardResult::Redact is now the permanent implementation, not a temporary workaround. Signed-off-by: mkoushni <mkoushni@redhat.com> * fix(guardrails): update nemo-guardrails.yaml comment for modified verdict The modified verdict now rejects with 403 (same as blocked). Update the example config comment to reflect the current behaviour instead of the stale deferred-to-praxis-proxy#579 note. Signed-off-by: mkoushni <mkoushni@redhat.com> * test(guardrails): document helper and drop rejection-body clone Move nemo_pii_redact_filter next to the other test helpers with a doc comment, and read the rejection body with as_deref() instead of clone(). Signed-off-by: mkoushni <mkoushni@redhat.com> * test(guardrails): backtick NeMo in helper docs for clippy clippy::doc-markdown treats NeMo as a missing identifier; wrap it so lint CI can compile praxis-ai-filters tests. Signed-off-by: mkoushni <mkoushni@redhat.com> --------- Signed-off-by: mkoushni <mkoushni@redhat.com>
* feat(llm-d): move ext_proc compatibility into AI Move the llm-d ext_proc compatibility layer into the AI repository under integrations/llmd/ext-proc and gate runtime registration behind the llmd-ext-proc feature. The crate is publish=false and scoped to llm-d's current EPP protocol rather than general-purpose Envoy ext_proc support. Environment tests cover the mock EPP to endpoint_selector to llm-d-inference-sim path. Use the published Praxis 0.4.1 crates now that the required support APIs are available from crates.io. Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> * docs: use ascii arrows in llm-d testing guide Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> * docs: use ascii punctuation in llm-d transfer Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> * fix(llm-d): address ext_proc review feedback Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> * test(llm-d): share ext_proc routing mock Move the duplicated mock routing processor into the feature-gated test utilities module. Reuse it from the example and simulator environment suites while keeping the gRPC dependencies scoped to llmd-ext-proc tests. Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> * fix(llm-d): align rebased test support Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> * fix(llm-d): harden ext-proc boundary handling Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> --------- Signed-off-by: Brent Salisbury <bsalisbu@redhat.com> Co-authored-by: Morgan Foster <39788015+usize@users.noreply.github.com>
…raxis-proxy#760) * feat: http_callout using SubRequestConnector Signed-off-by: Gabriela Dozortsev <gdozorts@redhat.com> Signed-off-by: usize <mofoster@redhat.com> * control character parsing for llama guard Signed-off-by: Gabriela Dozortsev <gdozorts@redhat.com> Signed-off-by: usize <mofoster@redhat.com> * chore: regenerate Cargo.lock after rebase onto main The lockfile on the PR branch carried dangling references to thiserror 2.0.19 (no matching package entry), which panicked cargo-audit in the security-audit and dependency-check jobs. Reset to main and re-resolve so only the serde_json_path additions remain. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * feat(callout): scope examples to Lakera Guard Defer the LlamaGuard example to a follow-up: its verdict format ("unsafe\nS02", "unsafe01".."unsafe13") needs the richer on_result matching proposed in praxis-proxy/praxis#964. Lakera Guard returns exact "true"/"false", which works with exact-equality matching today, and is the guardrails integration required by the AI Gateway MVP (ai#758, success criterion 4). Move lakera-guard.yaml back to examples/configs/ (the subdirectory only existed to group the two guard examples) and regenerate the examples README table. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): resolve clippy lints and rustfmt drift Refactor the callout filter to satisfy the workspace lint gates (clippy -D warnings, nightly rustfmt): - introduce a CalloutTarget struct in place of the six target_* fields and the parse_callout_target six-tuple, splitting scheme and host validation into helpers; - extract callout header assembly, response handling, the network round-trip, depth parsing, and the max_body_bytes bound check into focused helpers to clear too_many_lines/cognitive_complexity and large_stack_frames on execute_callout; - document and split sanitize_string, replace string indexing with checked slicing, and neutralise its LlamaGuard-specific warning; - hoist DISALLOWED_FORWARD_HEADERS to a module const; - assert http_callout registration in the AI registry test. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * docs(callout): make filter visible to docs generator and add reference docs cargo xtask generate-filter-docs discovers filter anchors by the string literal returned from name(); returning the FILTER_NAME const made http_callout invisible, so docs/filters/http_callout.md was never generated (AGENTS.md test requirement praxis-proxy#5). Return the "http_callout" literal directly, keep FILTER_NAME for internal use with a drift test, lead the struct doc with a descriptive summary line, and check in the generated reference docs. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): inject response headers with set semantics Inject callout response headers via ctx.request_headers_to_set (overwrite) instead of ctx.extra_request_headers (append), so a header taken from the trusted callout response replaces any client-supplied header of the same name rather than being appended alongside it. Also drops a lossy HeaderValue::to_str() conversion by pushing the HeaderValue directly. Adds a test that an inject header absent from the response is not injected. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): validate result_key at config time; skip rejected values P3: CompiledExtraction::compile now probes result_key against the FilterResultSet key rules (ASCII alphanumeric/_/- , 1-64 bytes) with an empty value, so invalid keys such as "lakera.flagged" or an empty key fail at startup rather than silently on every request. P4: evaluate no longer returns Result. A coerced value rejected by the result-set limits is logged (warn) and skipped instead of propagating through handle_success and failing the whole request via ?, so an oversized/hostile third-party response value is handled per the on_failure policy. handle_success/handle_response drop their now unnecessary Result wrappers. Note: with the key validated at config time and sanitize_string capping string/array/object coercions at 255 bytes (below the 256 value limit), the value-rejection branch is defense-in-depth and not reachable through the current coercion pipeline; it degrades gracefully if those limits ever change. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): remove dead failure_mode alias on on_failure The #[serde(alias = "failure_mode")] on on_failure could never bind: core strips failure_mode as a structural pipeline key (see praxis-proxy-filter factory strip_structural_keys) before the filter config is parsed, and it controls a different semantic (how the pipeline reacts to a filter *error*, not how this filter reacts to a *callout* failure). Remove the misleading alias and document the distinction. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * test(callout): port coverage gaps from earlier review rounds Add the test cases that earlier review rounds surfaced but that had not made it into the scoped branch: - forward_header_absent_from_request_not_sent: a configured forward_header that is absent from the downstream request is not sent to the callout (asserts against the mock's received requests). - body_shaping_non_json_forwards_raw: when body shaping is configured but the downstream body is not JSON, the raw body is forwarded verbatim rather than dropped, and extraction still succeeds. - config_rejects_unknown_{target,response,circuit_breaker}_field: deny_unknown_fields is enforced on the nested config structs, not just the top level. - lakera_guard_get_bypasses_callout (integration): the example scopes the callout to methods: [POST], so a GET reaches the upstream without a callout even when Lakera would flag it. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * docs(callout): regenerate http_callout doc for on_failure description The generated field table lagged the source doc comment on `on_failure` after the dead `failure_mode` alias was removed. Regenerate so `docs/filters/http_callout.md` matches the config source and passes `cargo xtask lint-filter-docs`. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): validate status_on_error is a legal HTTP status praxis-bot review (praxis-proxy#760): status_on_error accepted any u16, so values like 0, 99, or 65535 would produce a nonsensical HTTP status on the rejection path. Validate it falls in 100-599 at config time, alongside the existing max_body_bytes check; an unset value still defaults to 403. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * docs(callout): note status validation duplication for follow-up The 100..=599 status check is duplicated across openai_responses_compact, web_search, and now callout. Record the known duplication and the plan to promote a shared helper into praxis-ai-apis so the follow-up dedupe is discoverable from the code. No behavior change. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * test(callout): cover non-2xx callout response forwarding A completed callout that answers with a non-2xx status forwards that status to the downstream client, which is distinct from a transport failure applying status_on_error. The new test mounts a mock returning 500 with on_failure: open and asserts Reject(500) — proving the non-2xx branch forwards the callout's own status regardless of failure mode. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * test(callout): cover userinfo-in-URL rejection parse_host rejects URLs containing '@' to prevent embedded credentials from leaking into logs or being forwarded to the callout target. Add a test asserting both user:pass@host and bare user@host are rejected at config time with an error that mentions userinfo. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): stop stripping slashes from extracted values split_at_first_control removed '/' and '\\' from every extracted value, mangling legitimate results ("unsafe/S02" -> "unsafeS02", "safe/clean" -> "safeclean") and silently breaking on_result matching. The stripping was incidental to the original control-character work for llama guard and served no security purpose: control-character truncation already defends against CR/LF/header-injection, and slashes are ordinary value characters. Preserve non-control characters verbatim and split at the first control character only. Add tests pinning slash preservation and confirming a control character still truncates. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * test(callout): cover https target parsing (TLS/SNI/port) CalloutTarget::parse had no coverage of the https path. Add tests asserting https enables TLS, sets SNI to the host, defaults the port to 443 and omits it from the Host authority; that a non-default https port is kept in the authority while SNI stays host-only; and that http disables TLS, defaults to port 80, and leaves SNI empty. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * feat(callout): warn on disallowed forward_header at config time Hop-by-hop and sensitive headers in DISALLOWED_FORWARD_HEADERS are silently skipped at request time, so an operator who lists one as a forward_header gets no feedback that it is a no-op. Emit a warning per such header at config time; the entry remains non-fatal and the request-time skip is unchanged as defense-in-depth. Add a test that a disallowed forward_header is accepted (warns, not errors) and a wiremock test proving a disallowed header is not sent to the callout while an allowed one is. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): cap response body at configured max_body_bytes build_subrequest_client used SubRequestClient::new, leaving the client response-byte ceiling at its 64 MiB default. Because execute() uses min(per_call_limit, client_ceiling), a configured max_body_bytes above 64 MiB was silently clamped to 64 MiB. Construct the client with with_max_response_bytes(connector, max_body_bytes) so the effective response limit always equals the operator's configured value. Add a test that a response body larger than max_body_bytes fails the callout (Reject(status_on_error) under on_failure: closed). Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * style(callout): apply nightly rustfmt to fixup commits Wrap two over-width lines flagged by nightly rustfmt in the status_on_error validator error and the https target-parse assertion. No behavior change. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): block resolved private/loopback peers (SSRF/rebinding) Add opt-in `target.allow_private_addresses` (default true, preserving current warn-only behavior). When false, resolve_peer rejects a resolved private/loopback/link-local peer after DNS resolution, closing the DNS-rebinding gap that the config-time literal-IP check cannot catch (e.g. a hostname resolving to 169.254.169.254). A blocked peer is treated as a callout failure and follows on_failure. Defer to the shared classifier praxis_core::connectivity::is_private_ip rather than adding another hand-rolled private-address predicate; see praxis-proxy#771 for unifying the existing copies. Harden the lakera-guard example accordingly. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(agentic): gate parse_json_rpc_body body arg for praxis-core main praxis-core `main` changed the first parameter of `parse_json_rpc_body` from `&Option<Bytes>` (released 0.5.2) to `Option<&Bytes>`, which breaks the A2A and MCP filters under the `test-praxis-main` compatibility job (E0308) at a2a/mod.rs:178 and mcp/mod.rs:148. This is upstream drift caught by the forward-looking canary, not a defect in this branch: the default build pins praxis-core 0.5.2 and stays green, while `test-praxis-main` clones core `main` at HEAD. Gate the call argument on the existing `praxis-main` feature, matching the pattern already used in inference/model_to_header.rs: pass `&*body` against 0.5.2 and `body.as_ref()` against core `main`. A TODO marks the gate for removal once we pin to a praxis-core release that ships the new signature. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * docs(callout): note max_body_bytes also caps callout response body request.max_body_bytes is passed both to the forwarded-request buffer and to SubRequestClient::with_max_response_bytes (build_subrequest_client) and the per-request execute limit, so it also bounds how large a callout response the filter will accept. The field doc only described the request role; note the dual role and regenerate the reference doc. Addresses praxis-bot review comment on filters/src/callout/config.rs. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): box-pin large callout futures to satisfy clippy The praxis-proxy-filter 0.5.3 API changes (merged from main) enlarged the SubRequestClient::execute future, pushing the callout await chain over clippy's large_futures / large_stack_frames thresholds under -D warnings. Box::pin the awaited futures at each flagged site (client.execute, perform_callout, and the execute_callout calls in on_request / on_request_body), matching the fix main applied to apis/src/subrequest.rs in 5b45cc1. Moves the future to the heap so the stack frame stays small. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * test(server): opt dump loopback fixtures into allow_private_endpoints praxis-core main added validation of clusters defined inline in load_balancer filters (praxis 894ac183), so a loopback cluster endpoint like 127.0.0.1:9090 is now rejected at config-parse time unless insecure_options.allow_private_endpoints is set. This broke the two credential_injection dump tests under the test-praxis-main canary (server/src/dump.rs config-parse panic), while the released 0.5.3 build stays green. The failing tests are unrelated to their subject (credential redaction); they panic earlier during Config::from_yaml. Add the same insecure_options.allow_private_endpoints: true opt-in that praxis-core applied to its own loopback fixtures (praxis 185545ae). The field exists in released 0.5.3, so the default build is unaffected. Verified: both tests pass against released 0.5.3 and against core main (--features praxis-main); full server bin, filters, and apis suites pass against core main. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * test(callout): opt lakera-guard example into allow_private_endpoints praxis-core main validates clusters defined inline in load_balancer filters and rejects private/loopback/link-local endpoints unless the config sets `insecure_options.allow_private_endpoints: true` (core commit 894ac18). praxis-proxy#800 added this opt-in to every shipped example, but the lakera-guard example was introduced on this branch and so was missed, leaving `test-praxis-main` red on the schema parse_configs check: examples/configs/lakera-guard.yaml: chain 'routing': filter 'load_balancer': cluster 'backend': endpoint '127.0.0.1:3000' resolves to a sensitive address; set insecure_options.allow_private_endpoints: true to allow Mirror praxis-proxy#800's remediation: append the top-level insecure_options block. The flag ships in released praxis 0.5.3, so this is green on both the pinned dependency and core main. The lakera integration test builds its config manually (not via the allow_loopback_endpoints helper), so it inherits the flag from the file with no duplicate-key collision. Assisted by Opus 4.8 Signed-off-by: usize <mofoster@redhat.com> * fix(callout): trim trailing whitespace and correct the sanitize docs The `sanitize_string` docstring claimed it "drops `/` and `\` from the retained text". It does not, and has not since slash stripping was removed — `split_at_first_control` splits only on control characters, and its own doc says slashes are preserved verbatim. The two comments contradicted each other, which is what prompted the review question. Fixing the docstring surfaced a real gap next to it: leading whitespace was trimmed but trailing whitespace was not, so a provider returning `"safe "` produced `"safe "` and silently failed `on_result` exact-equality matching against a config saying `safe`. Trim both ends. The trim runs after the control-character split, so the `warn!` still reports the untrimmed dropped remainder and truncation still applies to the final value. Tests added: - trailing/surrounding whitespace is trimmed, interior spacing is not - whitespace sitting just before a control character is removed - whitespace-only input yields `None` - truncation at, over, and exactly at `MAX_SANITIZED_LEN` - the UTF-8 boundary walk, including 2-, 3-, and 4-byte characters, so a multi-byte character straddling byte 255 is never split - `coerce_value` over null, bool, number, string, array, and object Verified the four behavioral tests fail against the pre-fix implementation and pass after it. Assisted by Opus 5 Signed-off-by: usize <mofoster@redhat.com> * feat(callout)!: gate http_callout behind an experimental build flag Per review: the filter is a work in progress, so put a build flag around it and let it soak before it counts as supported surface. Adds `http-callout-filter` to praxis-ai-filters and praxis-ai-proxy, off by default. It activates an `experimental` marker feature, mirroring how praxis core's server crate buckets `basic-auth-filter` under `experimental`, so consumers can gate on "anything experimental" without naming each feature. Gated: the `callout` module and its `HttpCalloutFilter` re-export, the registration in `register_ai_filters`, and the `lakera-guard` example integration test (it runs the proxy in-process, so the filter must be compiled in). The registry test now asserts both directions — present with the feature, absent without it — so the gate cannot silently regress. Documented in the filter's struct doc, which flows into the generated `docs/filters/http_callout.md`, and in the example config's usage line. Verified both configurations: default build 1073 filter tests pass with no callout code compiled in; with the feature, 1172 pass plus the three lakera integration tests. Clippy clean both ways; full workspace suite green. BREAKING CHANGE: `http_callout` is no longer registered in a default build. Enable `--features http-callout-filter` to use it. Assisted by Opus 5 Signed-off-by: usize <mofoster@redhat.com> --------- Signed-off-by: Gabriela Dozortsev <gdozorts@redhat.com> Signed-off-by: usize <mofoster@redhat.com> Co-authored-by: Gabriela Dozortsev <gdozorts@redhat.com>
…xis-proxy#802) * aws: add aws_sigv4_sign filter config scaffold Signed-off-by: szedan <szedan@redhat.com> * aws: implement sigv4 canonical-request signing, verified against AWS's published test vector Signed-off-by: szedan <szedan@redhat.com> * aws: implement Sigv4SignFilter HttpFilter, wired to sign_headers Signed-off-by: szedan <szedan@redhat.com> * aws: register aws_sigv4_sign as a security-class filter Signed-off-by: szedan <szedan@redhat.com> * aws: add aws-sigv4 example config and functional integration test Signed-off-by: szedan <szedan@redhat.com> * xtask: title-case the AWS filter-doc category as AWS, not Aws Signed-off-by: szedan <szedan@redhat.com> * aws: fix broken intra-doc links and reformat with workspace nightly rustfmt Full verification (make lint / make doc) surfaced issues not caught by stable cargo fmt/clippy alone: unresolved rustdoc links to praxis_filter::HttpFilterContext (out of scope in this module), and three functions that crossed the 30-line clippy::too_many_lines threshold once reformatted with the workspace's nightly rustfmt settings. Signed-off-by: szedan <szedan@redhat.com> * aws: generate aws_sigv4_sign filter reference docs Signed-off-by: szedan <szedan@redhat.com> * aws: opt aws-sigv4 example into allow_private_endpoints Praxis core now rejects inline load_balancer clusters that target loopback/private endpoints unless the config sets insecure_options.allow_private_endpoints: true. The aws-sigv4 example proxies to a local test double (127.0.0.1:3000), so opt it in the same way upstream praxis-proxy#800 did for the other shipped example configs. Signed-off-by: szedan <szedan@redhat.com> * aws: harden aws_sigv4_sign per review feedback - Log signing failures with tracing::warn before failing closed with 503, so operators can distinguish misconfiguration from an outage. The error carries only signing-input diagnostics, never credential material. - Validate the configured host as a HeaderValue once in new() and reuse it, moving the check to startup and off the per-request path. - Reject max_body_bytes of 0 or above a 64 MiB ceiling at construction. - Add tests for the runtime signing-failure 503 path and the max_body_bytes bounds; extract credential resolution into a helper. Signed-off-by: szedan <szedan@redhat.com> * aws: split registry test to satisfy too_many_lines lint The upstream merge combined the aws_sigv4_sign and http_callout assertions into build_ai_registry_includes_ai_and_builtin_filters, pushing it one line over the 30-line clippy limit. Move the security-filter assertions into a focused test. Signed-off-by: szedan <szedan@redhat.com> --------- Signed-off-by: szedan <szedan@redhat.com>
…raxis-proxy#702) * refactor(responses): centralize usage accumulation Signed-off-by: kaplan <rivka.kaplan@nokia.com> Signed-off-by: kaplan <rkaplan@redhat.com> * style: apply rustfmt to responses usage modules Signed-off-by: kaplan <rkaplan@redhat.com> --------- Signed-off-by: kaplan <rivka.kaplan@nokia.com> Signed-off-by: kaplan <rkaplan@redhat.com>
…ing usage (praxis-proxy#782) * fix(token_usage): recover from SSE overflow instead of silently dropping usage Token accounting stopped permanently after a single oversized SSE event or JSON response, clearing all working state (including the terminal usage event) with no signal that data was lost. - The shared SSE scanner (used by token_usage and a2a) now discards only the oversized event and resumes at the next event boundary, instead of aborting the whole stream. A terminal usage event arriving after an oversized one is now captured. - JSON and any residual SSE overflow (e.g. the usage event itself being oversized) now set an explicit token.status=overflow metadata key and Praxis-Token-Status response header, so billing consumers cannot mistake missing counts for zero usage. - max_body_bytes/max_scratch_bytes are now configurable per token_count filter instance instead of fixed at 1 MiB / 64 KiB. Fixes praxis-proxy#674 Signed-off-by: mkoushni <mkoushni@redhat.com> * fix(token_usage): share SkipPhase encoding and reject zero capture limits Give A2A and token_count a single SkipPhase metadata codec, reject zero max_body_bytes/max_scratch_bytes, and regenerate filter docs so lint CI matches the new config fields. Signed-off-by: mkoushni <mkoushni@redhat.com> * fix(token_usage): mark overflow when a dropped SSE event is the tail Partial Anthropic/Bedrock counts followed by an oversized terminal usage event now keep the captured maxima and set token.status=overflow, while a recovered usage event after a drop stays authoritative. Signed-off-by: mkoushni <mkoushni@redhat.com> * fix(token_usage): cap max_scratch_bytes at the shared 64 MiB ceiling Signed-off-by: mkoushni <mkoushni@redhat.com> --------- Signed-off-by: mkoushni <mkoushni@redhat.com>
praxis-bot
left a comment
There was a problem hiding this comment.
PR Re-Review (4 new commits)
Summary: Re-reviewed the token_rate_limit filter, picker module, group_index, and overlay selection policy changes. The Lua key-scoping bug and enqueue_reconcile error propagation from the previous review remain unaddressed. One new finding below.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 0 |
| Medium | 1 |
| return Ok(FilterAction::Continue); | ||
| }; | ||
| if let Some((ledger, _)) = rule.backend.local_state() { | ||
| let _ = ledger.reconcile(id, actual, self.now_ms()); |
There was a problem hiding this comment.
[Medium] The in-memory reconciliation path in on_response_body discards the Settlement result with let _ =, so none of the reconciliation counters are recorded for successful responses. Compare with self.reconcile() (lines 416-424), which records reservations_total{result="reconciled"}, tokens_total{kind="actual"}, tokens_total{kind="refunded"}, and tokens_total{kind="overage"}, and calls self.record_state_metrics() to update active-reservation and key gauges.
For the in-memory backend, successful 2xx responses take this path (the most common reconciliation path). Non-success responses go through on_response which calls self.reconcile() with metrics. This means operators using the in-memory backend will see reservations_total{result="created"} climbing while reservations_total{result="reconciled"} stays flat for successful traffic, making it appear that reservations are leaking.
Match on the Settlement return value and record the same counters:
match ledger.reconcile(id, actual, self.now_ms()) {
Settlement::Applied { actual, refund, overage } => {
counter!("praxis_ai_token_rate_limit_reservations_total", "result" => "reconciled").increment(1);
counter!("praxis_ai_token_rate_limit_tokens_total", "kind" => "actual").increment(actual);
counter!("praxis_ai_token_rate_limit_tokens_total", "kind" => "refunded").increment(refund);
counter!("praxis_ai_token_rate_limit_tokens_total", "kind" => "overage").increment(overage);
},
Settlement::Noop => {},
}
self.record_state_metrics();Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Keep the token quota branch buildable against the corrected Praxis identity\nmetadata API and current AI filter lifecycle. Preserve Basic Auth, token counting,\nprovider routing, and the existing bounded-route coverage while updating the\ncompatibility paths required by the stacked dependency. Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Use the published Praxis dependency in normal CI. The compatibility workflow\ncontinues to apply its checked-out Praxis source explicitly, avoiding duplicate\npatch tables and personal Git sources in supply-chain validation. Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
fb9a89d to
3bac9f3
Compare
|
No longer needed for the TRL reference. |
Summary
Dicalimer 🔴 This is not for merge, but a fairly cleaned up effort at a groundwork for quotas used for this demo: Distributed token quota demonstration: grid-distributed-token-rate-limit:
https://github.com/praxis-proxy/experimental/tree/main/demos/grid-distributed-token-rate-limit
This PR adds authenticated, model-scoped token quota enforcement to Praxis AI through the new token_rate_limit filter.
The filter admits or rejects a request before provider routing begins. It reserves an estimated token amount against one or more sliding-window budgets, then reconciles that reservation with the actual token usage reported after the response completes.
The quota mechanism is independent of Grid routing:
It can therefore be used with Grid provider selection, static upstream routing, or another routing implementation. Grid does not store quotas, contact the quota backend, or participate in quota enforcement.
Project Stack
The ownership boundaries are intentional:
Quota enforcement remains outside Grid and does not add a quota lookup to Grid reconciliation or provider selection.
Related Work
AI token rate-limiting proposal: docs/proposals/00121_token-rate-limiting.md
https://github.com/praxis-proxy/ai/blob/main/docs/proposals/00121_token-rate-limiting.md
AI token-rate-limiting implementation: docs(proposal): add implementation to token rate-limiting #658
docs(proposal): add implementation to token rate-limiting #658
Praxis authenticated-principal metadata establishes the trusted identity.user_id contract consumed by this filter.
AI provider-selection foundation: feat(routing): add provider load balancing, feat(routing): add provider load balancing #731
feat(routing): add provider load balancing #731
Grid provider-selection groups: feat(routing): publish provider selection modes, feat(routing): publish provider selection modes grid#65
feat(routing): publish provider selection modes grid#65
Distributed token quota demonstration: grid-distributed-token-rate-limit
https://github.com/praxis-proxy/experimental/tree/main/demos/grid-distributed-token-rate-limit
AI PR #731 supplies the provider-selection and load-balancing foundation. The distributed quota behavior is demonstrated separately by the experimental demo.
Behavior
The filter performs the following request lifecycle:
Admission happens before routing. A rejected request therefore cannot select a provider or consume provider capacity.
Quota Keys
The current quota key is composed from two bounded inputs:
authenticated principal + configured model
For example:
The principal must come from trusted request metadata established by an authentication filter. The limiter does not trust a client-provided identity header and does not parse credentials itself.
The model is read from a configured request header and must match the configured model allowlist.
This provides independent quota accounting for combinations such as:
The implementation is suitable for non-Grid deployments as long as the pipeline provides:
Sliding-Window Budgets
Each rule can define multiple token budgets. Admission reserves capacity atomically across every configured window.
Example:
The request is admitted only when every applicable window has sufficient capacity.
The implementation uses one-second accounting buckets with explicit bounds on:
Sliding-window recovery occurs naturally as prior usage ages out of the configured window. It does not require clearing the backend or restarting a gateway.
Reservation and Reconciliation
Admission uses a fixed token estimate so the decision can be made before inference begins.
After the response completes, the reservation is reconciled with actual token usage:
This avoids admitting unlimited concurrent requests based only on completed usage.
Backends
In-Memory
The in-memory backend provides exact local sliding-window accounting for one gateway process.
It is appropriate for:
Its state is not shared across gateway replicas and does not survive process restart.
Valkey
The Valkey backend provides shared quota enforcement across multiple gateway processes.
Both gateways reserve against the same principal-and-model budget. A request through one gateway therefore reduces the capacity visible through the other gateway.
The backend uses:
Valkey URLs can be supplied through an environment-variable reference so credentials do not need to appear directly in the filter configuration.
rediss://is supported when the deployment provides the required trust roots and validates the server certificate. A password or private cluster network alone does not encrypt Valkey traffic.Routing Independence
The filter does not depend on:
A non-Grid deployment can use:
A Grid-aware deployment can use:
Pipeline validation ensures that, when these filters are present:
Provider selection can change between requests without changing the quota key. With the Valkey backend, quota also remains consistent when requests arrive through different gateway replicas.
Responses
When quota capacity is exhausted, the filter returns HTTP 429 before routing.
The response includes bounded rate-limit information such as:
Denied requests do not contain provider attribution because provider selection never occurred.
Backend failures use a distinct fail-closed service response rather than admitting requests without accounting.
Missing identity, missing model, unknown model, oversized keys, missing rules, and state-capacity failures are handled explicitly rather than silently bypassing quota enforcement.
Configuration Example
Security Considerations
The filter consumes an authenticated identity; it does not establish one.
identity.user_id must be written only by a trusted authentication filter after successful verification. Clients must not be able to set or override this metadata directly.
The implementation does not place the following in quota metadata, metric labels, error bodies, or logs:
Principal and model values are bounded before allocating state. Metrics use bounded result, operation, token-kind, and backend labels rather than raw user or model identifiers.
Valkey credentials should be supplied through a Secret-backed environment variable. Production deployments should use encrypted and authenticated transport.
Performance and Hot-Path Behavior
The filter adds quota admission to the request path, so its backend choice defines the operational tradeoff.
In-Memory Backend
The local backend performs bounded in-process accounting and synchronization. It does not make network, Kubernetes, filesystem, Grid, or metrics-service calls.
Valkey Backend
The shared backend performs a Valkey operation during admission so multiple gateways can enforce one quota atomically. This is an intentional distributed consistency boundary.
The implementation limits its cost through:
No Grid call, Kubernetes lookup, provider-health query, Prometheus scrape, EPP request, or overlay parsing is introduced by quota enforcement.
Quota denial occurs before provider routing and inference, avoiding unnecessary downstream work.
Observability
The filter records bounded operational metrics for:
Raw principals and complete quota keys are not used as Prometheus labels.
The distributed demo additionally presents request-level quota decisions, selected providers for admitted requests, pre-provider 429 behavior, shared enforcement across gateway replicas, and sliding-window recovery.
Compatibility
The filter is additive and must be explicitly included in a pipeline.
Deployments that do not configure token_rate_limit retain their existing behavior.
The implementation does not change:
The filter can be introduced independently of Grid. A Grid-aware deployment requires the provider-selection work only for the demonstrated round-robin routing behavior, not for quota enforcement itself.
Current Scope
This PR implements:
The current scope does not yet provide:
Those capabilities can build on the same authenticated-identity and backend contracts without coupling quota enforcement to provider routing.
Validation
Validation covers:
The distributed demonstration proves:
Checklist
Breaking Changes
No breaking changes are intended.
The filter is opt-in. Existing listeners and filter pipelines remain unchanged unless token_rate_limit is explicitly configured.