Skip to content

Commit 757dedf

Browse files
vvillait88claude
andcommitted
hardening: round-4 reviewer findings (warning scope + edge tests + README codes)
Tighten the EdDSA SecurityWarning suppression to the exact joserfc message and class so a future, unrelated EdDSA-vulnerability warning is no longer swallowed: scope is now `joserfc.errors.SecurityWarning` with literal regex `^EdDSA is deprecated via RFC 9864$` in both the helper and the pyproject `filterwarnings`. Add explicit canonicalization tests for NaN, positive-infinity, and negative-infinity floats (matching node-commerce parity). Add coverage for `unusable_key` (JWK with `use=enc`), non-string `signature` field (int / None / list / dict), non-dict JWKS entries, and JWS protected headers that decode to a JSON array. Extend the README error-code list with `unusable_key`, `malformed_jwks`, and `unrecognized_critical_header`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent afc7c3c commit 757dedf

4 files changed

Lines changed: 77 additions & 12 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ signed = sign_ucp_profile(profile.to_dict(), signing_key=key.private_key, kid=ke
226226
jwks = build_jwks_response([key.public_jwk])
227227
```
228228

229-
`verify_ucp_profile` enforces the JWS protected header `typ='ucp-profile+jws'`, restricts `alg` to `EdDSA`/`ES256`, requires a `kid`, rejects duplicate kids in the JWKS, and compares the canonical body bytes against the JWS payload to catch swap-after-sign tampering. Failures raise `UCPVerificationError` (a `ValueError` subclass) with a discriminated `code` attribute (`no_signature`/`missing_kid`/`kid_not_found`/`duplicate_kid`/`unsupported_alg`/`wrong_typ`/`signature_invalid`/`body_mismatch`/`malformed_jws`).
229+
`verify_ucp_profile` enforces the JWS protected header `typ='ucp-profile+jws'`, restricts `alg` to `EdDSA`/`ES256`, requires a `kid`, rejects duplicate kids in the JWKS, and compares the canonical body bytes against the JWS payload to catch swap-after-sign tampering. Failures raise `UCPVerificationError` (a `ValueError` subclass) with a discriminated `code` attribute (`no_signature`/`missing_kid`/`kid_not_found`/`duplicate_kid`/`unsupported_alg`/`wrong_typ`/`signature_invalid`/`body_mismatch`/`malformed_jws`/`malformed_jwks`/`unusable_key`/`unrecognized_critical_header`).
230230

231231
`sign_ucp_profile` rejects profiles containing `float` values: cross-language float canonicalization is not stable, so use decimal strings (e.g. `"9.99"`) for any monetary or fractional fields you put in `extras`.
232232

agentscore_commerce/identity/ucp_jwks.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,21 @@
4343

4444
@contextlib.contextmanager
4545
def _suppress_joserfc_eddsa_warning() -> Iterator[None]:
46-
"""Silence joserfc's per-call SecurityWarning for EdDSA.
46+
"""Silence joserfc's exact RFC 9864 EdDSA SecurityWarning.
4747
48-
joserfc treats EdDSA as "deprecated via RFC 9864" and emits a
49-
SecurityWarning every time we sign or verify. UCP §6 explicitly mandates
50-
EdDSA support so we suppress the warning at the call site rather than
51-
forcing every consumer to set warnings.filterwarnings globally. CI runs
52-
with -W error would otherwise fail noisily on every signing test.
48+
UCP §6 requires EdDSA support and joserfc emits a per-call deprecation
49+
warning. The filter is pinned to the exact message + class
50+
(``joserfc.errors.SecurityWarning``: ``"EdDSA is deprecated via RFC 9864"``)
51+
so a future, unrelated EdDSA warning still surfaces normally.
5352
"""
53+
from joserfc.errors import SecurityWarning # type: ignore[import-not-found]
54+
5455
with warnings.catch_warnings():
55-
warnings.filterwarnings("ignore", message=r".*EdDSA.*", category=Warning)
56+
warnings.filterwarnings(
57+
"ignore",
58+
message=r"^EdDSA is deprecated via RFC 9864$",
59+
category=SecurityWarning,
60+
)
5661
yield
5762

5863

pyproject.toml

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,10 @@ include = ["agentscore_commerce"]
8181
asyncio_mode = "auto"
8282
addopts = "--cov=agentscore_commerce --cov-report=term-missing --cov-fail-under=95"
8383
# joserfc emits SecurityWarning for EdDSA on every sign/verify (see RFC 9864).
84-
# UCP §6 explicitly accepts EdDSA so we suppress; the SDK helpers wrap their
85-
# own joserfc calls with a context-manager filter, but tests that hand-construct
86-
# joserfc calls directly need the suppression too.
84+
# UCP §6 explicitly accepts EdDSA so we suppress. SDK helpers wrap their own
85+
# joserfc calls with a context-manager filter, but tests that hand-construct
86+
# joserfc calls directly need the suppression too. Scope matches the exact
87+
# message + class so a future unrelated EdDSA warning still surfaces.
8788
filterwarnings = [
88-
"ignore:.*EdDSA.*:Warning",
89+
"ignore:^EdDSA is deprecated via RFC 9864$:joserfc.errors.SecurityWarning",
8990
]

tests/test_ucp_jwks.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,24 @@ def test_rejects_float_in_profile(self) -> None:
289289
with pytest.raises(ValueError, match="rejects float"):
290290
sign_ucp_profile(profile, signing_key=signer.private_key, kid="k")
291291

292+
def test_rejects_nan(self) -> None:
293+
signer = generate_ucp_signing_key(kid="k")
294+
profile = {**_base_profile([signer.public_jwk]), "extras": {"value": float("nan")}}
295+
with pytest.raises(ValueError, match="rejects float"):
296+
sign_ucp_profile(profile, signing_key=signer.private_key, kid="k")
297+
298+
def test_rejects_positive_infinity(self) -> None:
299+
signer = generate_ucp_signing_key(kid="k")
300+
profile = {**_base_profile([signer.public_jwk]), "extras": {"value": float("inf")}}
301+
with pytest.raises(ValueError, match="rejects float"):
302+
sign_ucp_profile(profile, signing_key=signer.private_key, kid="k")
303+
304+
def test_rejects_negative_infinity(self) -> None:
305+
signer = generate_ucp_signing_key(kid="k")
306+
profile = {**_base_profile([signer.public_jwk]), "extras": {"value": float("-inf")}}
307+
with pytest.raises(ValueError, match="rejects float"):
308+
sign_ucp_profile(profile, signing_key=signer.private_key, kid="k")
309+
292310
def test_accepts_int_and_string(self) -> None:
293311
signer = generate_ucp_signing_key(kid="k")
294312
profile = {**_base_profile([signer.public_jwk]), "extras": {"count": 7, "label": "wine"}}
@@ -362,6 +380,47 @@ def test_verify_rejects_non_dict_profile(self) -> None:
362380
verify_ucp_profile("not a profile", {"keys": []}) # type: ignore[arg-type]
363381
assert exc.value.code == "no_signature"
364382

383+
def test_verify_rejects_unusable_key_use_enc(self) -> None:
384+
key = generate_ucp_signing_key(kid="k")
385+
profile = _base_profile([key.public_jwk])
386+
signed = sign_ucp_profile(profile, signing_key=key.private_key, kid="k")
387+
enc_jwk = {**key.public_jwk, "use": "enc"}
388+
with pytest.raises(UCPVerificationError) as exc:
389+
verify_ucp_profile(signed, build_jwks_response([enc_jwk]))
390+
assert exc.value.code == "unusable_key"
391+
392+
@pytest.mark.parametrize("bad_sig", [42, None, [], {}])
393+
def test_verify_rejects_non_string_signature(self, bad_sig: object) -> None:
394+
key = generate_ucp_signing_key(kid="k")
395+
profile = _base_profile([key.public_jwk])
396+
tampered = {**profile, "signature": bad_sig}
397+
with pytest.raises(UCPVerificationError) as exc:
398+
verify_ucp_profile(tampered, build_jwks_response([key.public_jwk]))
399+
assert exc.value.code == "no_signature"
400+
401+
@pytest.mark.parametrize("bad_entry", [None, "string"])
402+
def test_verify_rejects_non_dict_jwks_entry(self, bad_entry: object) -> None:
403+
key = generate_ucp_signing_key(kid="k")
404+
profile = _base_profile([key.public_jwk])
405+
signed = sign_ucp_profile(profile, signing_key=key.private_key, kid="k")
406+
with pytest.raises(UCPVerificationError) as exc:
407+
verify_ucp_profile(signed, {"keys": [bad_entry]})
408+
assert exc.value.code == "kid_not_found"
409+
410+
def test_verify_rejects_protected_header_decoding_to_json_array(self) -> None:
411+
import base64
412+
413+
key = generate_ucp_signing_key(kid="k")
414+
profile = _base_profile([key.public_jwk])
415+
header_array_b64 = (
416+
base64.urlsafe_b64encode(__import__("json").dumps(["EdDSA", "kid"]).encode()).rstrip(b"=").decode()
417+
)
418+
bogus_jws = f"{header_array_b64}.payload.sig"
419+
signed = {**profile, "signature": bogus_jws}
420+
with pytest.raises(UCPVerificationError) as exc:
421+
verify_ucp_profile(signed, build_jwks_response([key.public_jwk]))
422+
assert exc.value.code == "malformed_jws"
423+
365424
def test_verify_wraps_unrecognized_critical_header(self) -> None:
366425
import base64
367426

0 commit comments

Comments
 (0)