Uh oh!
There was an error while loading. Please reload this page.
Refresh plugin dependencies and versions - #73
Merged
Conversation
lucarligforce-pushed
the
fix/refresh-plugin-dependencies
branch
from
May 1, 2026 11:43
b9f5e5c to
0967f1cComparelucarlig
marked this pull request as ready for review
May 1, 2026 12:57
Signed-off-by: lucarlig <luca.carlig@ibm.com>
lucarligforce-pushed
the
fix/refresh-plugin-dependencies
branch
from
May 1, 2026 14:33
0967f1c to
00c00a3Comparemsureshkumar88
approved these changes
May 1, 2026
msureshkumar88
left a comment
Collaborator
There was a problem hiding this comment.
Review
Verdict: Approve — no major issues. Changes are a clean lockfile refresh + consistent version bumps.
What this PR does
- Patch-bumps all 6 plugins (
encoded_exfil_detection,pii_filter,rate_limiter,retry_with_backoff,secrets_detection,url_reputation) - Refreshes
Cargo.lockanduv.lockwith compatible upstream updates (patch/minor bumps only;randandredismajor bumps intentionally deferred per PR notes) - Fixes
retry_with_backoff/Makefileto useuv run pythoninstead of barepython3— correctly enforces plugin-venv Python 3.11+ for coverage check - Updates CI fixture tag, catalog pinning, and catalog tests to match new versions
Verification
Version consistency — for all 6 plugins, Cargo.toml version and plugin-manifest.yaml version are in sync. No drift.
Cargo.lock notable bumps
| Crate | Old | New | Type |
|---|---|---|---|
pyo3 family | 0.28.2 | 0.28.3 | patch |
wasm-bindgen | 0.2.117 | 0.2.120 | patch |
tokio | 1.50.0 | 1.52.1 | minor |
rayon | 1.11.0 | 1.12.0 | minor |
indexmap | 2.13.0 | 2.14.0 | minor |
hashbrown | 0.16.1 | 0.17.0 | minor |
icu_* suite | 2.1.x | 2.2.0 | minor |
All within declared semver constraints. No unexpected major bumps.
wit-bindgen dual version (0.51.0 + 0.57.1 in lockfile) — expected, two separate transitive dep chains; not a concern.
Makefile fix — python3 $(REPO_ROOT)/tools/plugin_catalog.py → uv run python $(REPO_ROOT)/tools/plugin_catalog.py is the right fix. System Python 3.9 fallback would cause incorrect behavior for Python 3.11+ catalog checks.
Test updates — purely mechanical version-number updates tracking the new releases. No logic changes.
CI
CI is still running at time of review (all build-test and release-validation jobs queued/in-progress). Approve is contingent on CI green — please confirm before merging.
Minor note
PR description says "Standard detailed code review has not been run yet; run it before marking the PR ready." If that refers to an internal automated review tool, please run it before merge. The changes themselves are straightforward and low-risk.
Uh oh!
There was an error while loading. Please reload this page.
gandhipratik203 added a commit
that referenced
this pull request
May 1, 2026
Main bumped rate_limiter to 0.0.5 in #73 as part of a workspace-wide dependency refresh, so this PR's release slot moves to 0.0.6. The content of 0.0.6 is the TLS / rediss:// support work in this PR (crypto provider install, redis crate bump for advisory cleanup, TLS regression tests). Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
lucarlig pushed a commit
that referenced
this pull request
May 5, 2026
…is (#74) * fix(rate-limiter): enable TLS in redis client for rediss:// URLs The redis crate was compiled without any TLS feature, so passing a rediss:// URL (e.g. AWS ElastiCache with in-transit encryption, Redis Cloud) failed at backend init with `InvalidClientConfig: can't connect with TLS, the feature is not enabled`. The plugin manager then logged "Failed to load plugin RateLimiterPlugin" and silently skipped the plugin — bindings appeared configured but no rate limiting was applied. Compile the redis dep with rustls-based TLS (pure Rust, no OpenSSL): - tls-rustls - tls-rustls-webpki-roots (Mozilla CA roots, needed for managed Redis services without custom trust setup) - tokio-rustls-comp (async support over rustls) Added a regression test (TestRedisTlsSupport) that constructs a plugin with a rediss:// URL and asserts no InvalidClientConfig is raised. Bumps cpex_rate_limiter 0.0.4 → 0.0.5. Refs: wo-tracker #68217 Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com> * fix(rate-limiter): bump redis to 0.32 to drop unmaintained transitive dep cargo-deny security-policy CI was red on this branch with two findings: 1. RUSTSEC-2025-0134 — rustls-pemfile 2.2.0 is unmaintained. Pulled in by redis 0.27 → rustls-native-certs 0.7.3. 2. License rejected — webpki-roots is CDLA-Permissive-2.0, which is not in the workspace deny.toml allow-list. Both come from the TLS feature additions in the previous commit. Bumping the redis crate to 0.32 takes us to rustls-native-certs 0.8.x, which migrated to rustls-pki-types and dropped the rustls-pemfile dependency entirely. The advisory is closed at the root rather than suppressed via deny.toml. The license rejection is fixed by adding CDLA-Permissive-2.0 to the workspace allow-list. CDLA-Permissive-2.0 is a Linux-Foundation permissive data license used by webpki-roots for the Mozilla CA bundle — keeping the bundle pinned in the binary keeps deployments self-contained (no host-OS CA-store dependency in containers). No source-code changes — redis 0.27 → 0.32 is API-compatible for the APIs the Rust core uses (Client::open, MultiplexedConnection, cmd builder, redis::Value pattern matching with _ arms, ErrorKind variants we touch). All 47 redis call sites in redis_backend.rs compile unchanged. Local gate green: cargo check-all (57 Rust tests), pytest (112 Python tests). Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com> * test(rate-limiter): add TLS lazy-handshake regression test Addresses Luca's P2-4 review feedback on PR #74: the construction-only TLS regression test verifies URL parsing but not the lazy handshake. Added test_rediss_url_lazy_handshake_reaches_connectivity_layer that drives a tool_pre_invoke against rediss://127.0.0.1:1/15 so the lazy TLS path actually engages, then asserts the failure is connectivity- shaped (refused / timed out / IO) rather than the wo-tracker #68217 'feature is not enabled' / InvalidClientConfig signature. Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com> * fix(rate-limiter): install rustls crypto provider for rediss:// runtime path Two related changes that together make `rediss://` URLs actually work end-to-end: 1. Install the rustls ring crypto provider once at Rust core init. Previous TLS commits made `rediss://` URLs parse and the plugin load without the wo-tracker #68217 "feature is not enabled" error, but they left a runtime-time gap: rustls 0.23 dropped its implicit default crypto provider, so the first real TLS handshake panicked with `Call CryptoProvider::install_default()...`. Added a direct dep on rustls 0.23 with the `ring` feature, plus an OnceLock-guarded `ensure_crypto_provider()` in the Rust core that calls `rustls::crypto::ring::default_provider().install_default()` exactly once per process. Called from `RateLimiterPluginCore::new()` before any redis client is constructed. `ring` is the simpler default — pure Rust + small asm, BSD-3 / ISC licensed, builds cleanly across the existing PyPI wheel matrix without extra system toolchains. aws-lc-rs would be needed for FIPS validation; not a requirement for this customer. 2. Drop the `tls-rustls-webpki-roots` feature on the redis crate. That feature pulled `webpki-roots` (Mozilla's CA bundle, licensed CDLA-Permissive-2.0) into the dep tree, which the workspace deny.toml correctly rejected because that license is not in the allow list. Adding the license to the allow list (or as an exception) widens the workspace policy for one transitive crate. Without `tls-rustls-webpki-roots`, the redis crate uses `rustls-native-certs` (Apache-2.0 / MIT / ISC — already allowed) to read CA certificates from the host OS trust store at runtime. mcp-context-forge ships on UBI 10 Minimal, which includes `ca-certificates` by default; AWS ElastiCache uses public CA chains that are in any standard OS bundle. Operationally equivalent for this deployment, with a clean license posture. Reverts the `CDLA-Permissive-2.0` entry from the workspace deny.toml allow list — no longer needed. Test: the existing TLS lazy-handshake regression test (test_rediss_url_lazy_handshake_reaches_connectivity_layer) now reaches the network layer instead of panicking at TLS init. Its assertion is scoped to the wo-tracker #68217 negative signature ("feature is not enabled" / InvalidClientConfig must never appear). A positive "the lazy handshake actually reached the network" assertion would require the Rust core's log_exception() to surface the underlying RedisError text instead of the generic "error; allowing request" message — tracked as a follow-up alongside the real-TLS-Redis-fixture variant. Local gate: cargo fmt-check + clippy clean, 57 Rust unit tests pass, 107 Python integration tests pass. Cargo.lock confirms removal of unmaintained `rustls-pemfile` and license-flagged `webpki-roots`; `rustls-native-certs` 0.8.3 is the only remaining TLS-related dep. Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com> * chore(rate-limiter): bump 0.0.5 → 0.0.6 for TLS support release Main bumped rate_limiter to 0.0.5 in #73 as part of a workspace-wide dependency refresh, so this PR's release slot moves to 0.0.6. The content of 0.0.6 is the TLS / rediss:// support work in this PR (crypto provider install, redis crate bump for advisory cleanup, TLS regression tests). Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com> * test(rate-limiter): pin ensure_crypto_provider behaviour with a Rust unit test cargo-mutants flagged an uncovered mutation: replacing ensure_crypto_provider's body with () passed the existing test suite, because nothing in the Rust unit tests directly exercises the crypto provider install (TLS coverage lives on the Python integration side). Added a unit test in plugin.rs's tests module that calls ensure_crypto_provider() and asserts rustls::crypto::CryptoProvider::get_default().is_some(). - Real implementation installs the ring provider via OnceLock → get_default() returns Some → test passes. - Mutated implementation does nothing → no provider installed → get_default() returns None → test fails → mutant killed. OnceLock makes the call idempotent, so the test is order-independent within a cargo-test process. No behavioral change; pure coverage. Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com> * test(rate-limiter): add real TLS handshake regression test against a TLS-enabled Redis TestRedisTlsSupport only verified URL parsing and the absence of the wo-tracker #68217 InvalidClientConfig signature; nothing in that class drove a real handshake, so a regression in the rustls crypto provider, the cert-chain loader, or rustls-native-certs lookup could pass those tests yet fail at first contact with a real TLS server. This adds an end-to-end fixture that: - generates a self-signed CA + Redis server cert in ~/.cache/ - starts a redis:7 container with --tls-port 6390 (host port 16390) - exports SSL_CERT_FILE so both rustls-native-certs (Rust core) and Python's ssl module (test helpers) trust the test CA without modifying the OS trust store - drives tool_pre_invoke through rediss:// and asserts the counter key materializes in the TLS Redis — only a successful rustls handshake gets you a written key Skips cleanly when openssl or docker is unavailable, so laptops without docker stay green while Linux CI exercises the full path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com> * chore(rate-limiter): drop internal-tracker references from code comments Per review feedback: removes references to the internal customer issue tracker ("wo-tracker #68217") from code comments and docstrings. The technical descriptions of the regression are kept intact (e.g. "TLS-feature-not-enabled regression", "managed Redis with in-transit encryption", "InvalidClientConfig signature") so the comments still convey what each test is pinning; just the internal-only tracker ID is removed. Touches: - plugins/rust/python-package/rate_limiter/src/plugin.rs:553 - plugins/tests/rate_limiter/test_redis_integration.py — 6 mentions in TestRedisTlsSupport docstring/comments + TestRedisTlsHandshake docstring No behavioural change. All 59 tests pass. Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com> --------- Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Summary
Refs #69
Validation
Notes