feat(agent_card): add CARD-SIGN-003 protected header structural tests#196
feat(agent_card): add CARD-SIGN-003 protected header structural tests#196kuangmi-bit wants to merge 1 commit into
Conversation
…ation
Add reference vectors and structural tests for CARD-SIGN-003 (§8.4.2):
JWS protected header MUST include alg and kid parameters.
- card_signing_protected_header_vectors.json: 6 test cases covering
valid headers (ES256/RS256/EdDSA/ES384), missing kid, missing alg,
empty alg, and multi-signature cards
- test_card_signing_protected_header.py: two parametrized tests
- test_protected_header_has_alg_and_kid (MUST): base64url-decodes
protected header and validates alg + kid presence
- test_protected_header_alg_is_known_jws_algorithm (SHOULD):
advisory check against RFC 7518 §3.1 algorithm names
- Remove NOT_AUTOMATABLE tag from CARD-SIGN-003 since these are
offline structural checks requiring no SUT
12/12 tests pass, lint clean. No new dependencies (base64 + json
are stdlib).
This fixes a2aproject#187
There was a problem hiding this comment.
Code Review
This pull request automates the validation of the CARD-SIGN-003 requirement by removing the NOT_AUTOMATABLE tag and introducing test vectors and offline tests to verify that JWS protected headers contain valid alg and kid parameters. The review feedback points out potential AttributeError crashes in the test suite if a malformed JWS protected header decodes to a non-dictionary JSON type, and suggests adding explicit type checks in both _validate_protected_header and test_protected_header_alg_is_known_jws_algorithm to handle these cases gracefully.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _validate_protected_header( | ||
| case_id: str, header: dict | ||
| ) -> list[str]: | ||
| """Validate that a decoded JWS protected header satisfies CARD-SIGN-003. | ||
|
|
||
| Returns a list of error messages (empty list = valid). | ||
| """ | ||
| errors: list[str] = [] | ||
|
|
||
| alg = header.get("alg") |
There was a problem hiding this comment.
The _validate_protected_header function assumes that the decoded JSON header is always a dictionary. However, json.loads can return other valid JSON types (such as a list, string, number, boolean, or null) if the JWS protected header is malformed. If header is not a dictionary, calling header.get() will raise an AttributeError and crash the test suite. Adding an explicit type check ensures the test fails gracefully with a clear validation error.
| def _validate_protected_header( | |
| case_id: str, header: dict | |
| ) -> list[str]: | |
| """Validate that a decoded JWS protected header satisfies CARD-SIGN-003. | |
| Returns a list of error messages (empty list = valid). | |
| """ | |
| errors: list[str] = [] | |
| alg = header.get("alg") | |
| def _validate_protected_header( | |
| case_id: str, header: object | |
| ) -> list[str]: | |
| """Validate that a decoded JWS protected header satisfies CARD-SIGN-003. | |
| Returns a list of error messages (empty list = valid). | |
| """ | |
| errors: list[str] = [] | |
| if not isinstance(header, dict): | |
| errors.append(f"{case_id}: protected header is not a JSON object") | |
| return errors | |
| alg = header.get("alg") |
| alg = header.get("alg", "") | ||
| if alg and alg not in _KNOWN_JWS_ALGORITHMS: |
There was a problem hiding this comment.
In test_protected_header_alg_is_known_jws_algorithm, if the decoded header is not a dictionary (e.g., a list or string), calling header.get("alg", "") will raise an AttributeError outside of the try...except block. Adding a check to skip non-dictionary headers ensures the test behaves robustly, as structural non-dictionary errors are already handled by test_protected_header_has_alg_and_kid.
if not isinstance(header, dict):
continue
alg = header.get("alg", "")
if alg and alg not in _KNOWN_JWS_ALGORITHMS:|
Reviewed and independently validated the vectors. I decoded each of the 6 cases' Two things here are right and worth keeping:
One suggestion to complete the decode contract: add a negative where With #194 covering the 001/002 canonicalization payload and this covering 003 structural validation, the three automatable CARD-SIGN requirements are in good shape. Nice work. |
What
Adds reference vectors and structural tests for CARD-SIGN-003 (§8.4.2): the JWS protected header MUST include
alg(algorithm) andkid(key ID) parameters.Changes
card_signing_protected_header_vectors.json: 6 test casescardsign-hdr-001: valid ES256 headercardsign-hdr-002: valid RS256 headercardsign-hdr-003: missingkid→ negative casecardsign-hdr-004: missingalg→ negative casecardsign-hdr-005: emptyalg→ negative casecardsign-hdr-006: multi-signature card (ES384 + EdDSA)test_card_signing_protected_header.py: two parametrized teststest_protected_header_has_alg_and_kid(MUST): base64url-decodes each signature'sprotectedfield and validatesalg+kidare present, non-empty stringstest_protected_header_alg_is_known_jws_algorithm(SHOULD): advisory check against RFC 7518 §3.1 standard algorithm namestck/requirements/agent_card.py: RemoveNOT_AUTOMATABLEfrom CARD-SIGN-003Design decisions
base64+json. Nojwcryptoorrfc8785.kid, missingalg, and emptyalgto prove the validator catches violations._KNOWN_JWS_ALGORITHMSfrom RFC 7518) is a separate test so a non-standardalgproduces a warning-level failure rather than blocking the MUST check.Test results
make lintclean.uv run pytestgreen.Relationship to other PRs
NOT_AUTOMATABLEby design)This fixes #187