Skip to content

[Server] Pad EC key coordinates in the JWKS test fixture - #558

Merged
chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
mglaman:fix/flaky-ec-jwk-coordinates
Oct 8, 2026
Merged

chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
mglaman:fix/flaky-ec-jwk-coordinates

Conversation

@mglaman

@mglaman mglaman commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

testFromIssuerTagsKeysWithoutAlgorithmPerToken fails about 1 run in 150. It failed both unit (… lowest) jobs on #556, which doesn't touch OAuth.

The test builds a P-256 JWK from openssl_pkey_get_details(), which returns x and y without leading zero bytes. About 1% of generated keys have a 31-byte coordinate. RFC 7518 §6.2.1.2 requires the full 32 bytes. With --prefer-lowest (firebase/php-jwt 7.0.0), the key is rejected, and the ES256 token is refused.

The fixture now left-pads both coordinates:

$ecX = str_pad($ecDetails['ec']['x'], 32, "\0", \STR_PAD_LEFT);
$ecY = str_pad($ecDetails['ec']['y'], 32, "\0", \STR_PAD_LEFT);

Checked locally with lowest dependencies on PHP 8.5: 2 failures in 300 runs before, 0 in 600 after. In a separate script, 22 of 2,000 generated keys had a short coordinate.

🤖 Generated with Claude Code

OpenSSL returns P-256 coordinates without leading zero bytes, so about
1 in 100 generated keys has a 31-byte x or y. RFC 7518 requires 32
bytes, and with the lowest dependencies the key is rejected and
testFromIssuerTagsKeysWithoutAlgorithmPerToken fails.

🤖 Assisted with AI

@chr-hertel chr-hertel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, running into it right now as well - thanks!

@chr-hertel
chr-hertel merged commit 26b3f44 into modelcontextprotocol:main Oct 8, 2026
28 checks passed
@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants