fix: four gaps found porting Praxis onto this release - #72
Merged
Conversation
A name selector matches by glob and a route annotates its body under the pattern as written, but the lookup asked for the name the request arrived under. Those agree for an exact selector and diverge for a glob, so a route like `tool: "hr-*"` installed a handler and dispatched no policy: an operator wrote a deny and the request got an allow. Resolve the route and ask again under the name that matched, which is what the `http:` selector already did. Gated on the config declaring a glob, so an exact-only deployment pays nothing. Three tests pinned the old behavior as expected; they now assert the body runs, and the precedence and shadowing cases are covered beside them. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
The Rust API section is written for a host embedding the engine, and it named one of the four changes that break one. Add the HTTP hook family move, the Subject::claims shape change, and the HttpTransport a host must install, which is the only one of the four a clean build does not catch. Its perform_http half reaches a configuration rather than a host, so the introduction points a config-only reader at it. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
with_webpki_roots() builds its ClientConfig through rustls::ClientConfig::builder(), which reads the process-level CryptoProvider, and the host's dependency graph is what sets that. A graph carrying both ring and aws-lc-rs has none, so rustls panics on the first connection instead of choosing. That is praxis: pingora and its TLS stack pull aws-lc-rs while this transport pulls ring, so install_default_http_transport worked standalone and panicked in the gateway on the first JWKS fetch. Build the config against ring explicitly rather than installing a process default, which would race a host installing its own. client() is fallible as a result, since naming a provider means asking it for protocol versions. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
The per-hook reachability tally credited every plugin a route reaches with that route's CMF hook pair. A delegate step invokes its plugin under token.delegate and an elicitation verb under elicit, whatever entity the route selects, so a delegator came out covered on cmf.tool_pre_invoke and uncovered on the one hook it declares. plugin_narrowed_by_policy then fired on every config that delegates, which is most of them. The three demo policies raised four warnings and one was real. The plugin-level tally was already right, so this was noise rather than a load failure. The report still fires for a delegator declaring a CMF hook nothing reaches. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Comments, doc prose and assertion messages only. No logic, no dependency or feature changes. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
praxis-bot
reviewed
Sep 3, 2026
praxis-bot
left a comment
praxis-bot
left a comment
There was a problem hiding this comment.
praxis-bot review: fix/praxis-port-gaps
Purpose: Four independent fixes found while porting Praxis onto this release: glob route policy dispatch, hyper transport crypto provider panic, false plugin_narrowed_by_policy alarms, and upgrade guide gaps.
Assessment: Solid fixes with strong test coverage. Two medium findings.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 0 |
| Medium | 2 |
Test coverage is thorough: glob routes get five dedicated tests (match, multi-name, miss, exact-outranks-glob, exact-shadows-glob), delegator/elicitation hooks get three (covered, covered, unreached-still-reported), and the transport gets a process-default guard test. The existing matrix test was also updated to reflect the corrected behavior.
From the cargo-minor-patch dependabot PR (praxis-proxy#61), folded in here because it touches Cargo.lock and would otherwise conflict with this branch. Lock only, no manifest change. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Two review points on praxis-proxy#72. build_client kept the brace from the get_or_init closure it was extracted from. Removed; body is unchanged. The glob detector was asked to also treat `[` as a wildcard. It should not: wildmatch defines only `*` and `?` and no escapes, so `hr-[a-z]` is eight literal characters and matches nothing else. A literal selector needs no resolution, since its annotation key equals the request name whenever it matches. Recorded that in the function's doc and pinned it with a test. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
shaneutt
approved these changes
Sep 3, 2026
Member
|
Something weird happened and my comment didn't post: I noticed huge methods/functions, we should turn on the settings from conventions which blocks that, and restructure around functions. However, if you don't want to do that now, I consider this resolved if we create a follow-up issue to do this later. Once that's resolved, merge at your discretion 👍 |
Member
Author
|
Agreed, and we can address this in bulk when we close issue #15. Leaving this PR as is on that basis. |
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> # Conflicts: # Cargo.lock # crates/ppe/src/http_hyper.rs
github-project-automation
Bot
moved this from Review
to Done
in Praxis Policy Engine (PPE)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Four fixes found while porting Praxis onto this release. Each is independent.
Changes
A glob route never ran its policy. A route like
tool: "hr-*"resolved fine but itsauthorization:block was never dispatched, because the annotation is stored under the pattern and looked up under the request's name. An operator wrote a deny and the request got an allow. Three tests asserted the old behaviour; they now assert the new one.The bundled hyper transport panicked in Praxis. It built its TLS config from the process-level crypto provider, and Praxis's dependency graph has both
ringandaws-lc-rs, so rustls refused to pick one. It now namesringexplicitly instead of reading the ambient default.plugin_narrowed_by_policyfired on almost every config. A delegator runs undertoken.delegateand an elicitation handler underelicit, but both were credited with the route's CMF hook, so their real hook looked uncovered. The three demo policies raised four of these warnings and only one was real.The upgrade guide's Rust section covered one of four host-facing changes. Added the HTTP hook family move, the
Subject::claimsshape change, and theHttpTransporta host now has to install. The transport one matters most: it is the only one a clean build does not catch.Deps: Closes build(deps): bump uuid from 1.24.1 to 1.25.0 in the cargo-minor-patch group #61
Verified against the Praxis port and the policy-engine demo: 12 scenarios across Cedar, CEL and OPA, no load-time alarms.