fix(rest,ui): unbreak main — drop rsa from JWKS derivation, fix the stale keys test - #560
Merged
Conversation
…keys test The bulk-submit JWKS work brought in the pure-Rust rsa crate for RS384 public-JWK derivation, tripping the Security Audit on every PR: RUSTSEC-2023-0071 (Marvin Attack timing sidechannel) has no fixed release, and jwe.rs already rejects RSA-OAEP for exactly this reason. The JWKS handler's own doc promises an empty key set for non-ES384 algorithms -- the code now honors it. Both copies of the derivation (rest's derive_public_jwk, ui's signing_kid) return None for RS384 with the rationale in place; an RS384 deployment registers its key out-of-band, the assertion goes out without a kid, and the rsa crate leaves the lockfile. Also updates the UI keys test the redirect change left behind: the route permanently redirects to /.well-known/bulk-submit-jwks.json now, and the old 404/200 assertions were failing Test Rust on every PR's merge ref.
This was referenced Aug 18, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
smunini
approved these changes
Aug 18, 2026
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
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.
Main is red for every open PR since #539 merged — two independent breakages, both fixed here:
1. Security Audit fails on
rsa(RUSTSEC-2023-0071). #539 added the pure-Rustrsacrate in two places (rest'sderive_public_jwk, ui'ssigning_kid) for RS384 public-JWK derivation. The Marvin Attack advisory has no fixed release, and this repo already refuses the crate on principle —jwe.rsrejects RSA-OAEP citing exactly this advisory, and the JWKS handler's own doc comment promises "an empty keys array when … the configured algorithm is not ES384". The code now honors that contract: RS384 →Nonewith the rationale inline, ES384 derivation (via the existingp384) untouched,rsagone from both manifests and the lockfile. An RS384 deployment registers its key out-of-band and its assertions go out without akid— consistent with the endpoint serving no RSA key either.2. Test Rust fails on the stale keys test. #539 turned
/ui/bulk-import/keysinto aRedirect::permanentto/.well-known/bulk-submit-jwks.jsonbut left the old test asserting the 404/200 env-var contract — sothe_keys_endpoint_serves_the_configured_jwksfails (308 ≠ 404) on every merge ref. Replaced with a test of the actual contract: 308 + the Location header.Verified:
cargo tree -i rsaempty, both crates' suites green, fmt clean. Once this merges, re-running the checks on the open PRs (#538, #558) should clear them.