Repository navigation
fix(delegation,attestation): enforce RFC 8785 safe-integer bounds and canonical lowercase hex on HolderProof and CallerPossession (#274) - #277
Conversation
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
github-actions
Bot
added
the
needs-review:MEDIUM
carloshvp
left a comment
carloshvp
left a comment
There was a problem hiding this comment.
Reviewed exact head 252ac067f644908706c9dc34603d8ac947016975 in an isolated checkout.
Reviewed safe-integer checks at construction, parsing and evaluation boundaries, boolean exclusion, lowercase proof/MAC encodings and HolderProof's unknown-field rejection. The focused malformed-proof/numeric regression passes here and fails with the actual merge-base attestation/holder modules restored in process; valid holder-bound delegation tests remain passing.
Fresh hash-locked Python 3.12.10 validation: 781 unit tests passed, 16 skipped; pip check, Ruff, mypy, Bandit and diff whitespace checks passed. All commits have sign-offs. No blocking source finding in this scope; hosted checks and required security-maintainer approvals remain prerequisites.
Reviewed with OpenAI Codex.
252ac06 to
1889ce4
Compare
carloshvp
left a comment
carloshvp
left a comment
There was a problem hiding this comment.
Reviewed current head b5ff4de8b8b6acbb9faaad7a33d8e57c3c550bdf in an isolated checkout.
The scoped input-hardening behavior passes source review and the full local unit/conformance suite (837 passed, 16 skipped). Ruff lint, mypy, configured Bandit and dependency consistency pass; the commit is signed off.
The required ruff format --check src/ tests/ job still fails on this head, while the same command passes on current main. Please run the pinned formatter on src/ca2a_runtime/attestation.py, src/ca2a_runtime/delegation/revocation.py, tests/unit/test_holder_binding.py and rerun the format gate. The formatting diff is mechanical; no additional behavioral repair is requested by this review. Hosted CI and the independent security-review requirements still apply.
Reviewed with OpenAI Codex.
|
The requested formatting correction is ready for author adoption: dinakarjs#5, commit |
… canonical lowercase hex on HolderProof and CallerPossession (agentrust-io#274) - Bound depth, not_before, not_after, at_time, max_revocation_staleness, issued_at, and as_of by _MAX_SAFE_INTEGER (2**53 - 1) and validate depth in DelegationCredential.__post_init__. - Reject unknown fields and enforce 128-character lowercase hex on HolderProof.signature in HolderProof.from_dict and verify_holder_proof. - Reject unknown fields and enforce 64-character lowercase hex on CallerPossession.mac in CallerPossession.from_dict and verify_caller_possession. Closes agentrust-io#274 Signed-off-by: Raghuvar Karthik Durga <177662415+drk-ram@users.noreply.github.com>
|
@carloshvp Amended and force-pushed |
b5ff4de to
e7008a1
Compare
There was a problem hiding this comment.
Reviewed commit e7008a1881da68f0765b9deef3f3fe16fc9f2e38.
The requested formatting repair is complete. I compared the changed Python files with the revision from my prior review: their parsed ASTs are identical. The pinned repository formatter, Ruff, mypy and configured Bandit pass on this head. Focused tests: 35 passed (the HTTP replay case passed with loopback access after the sandbox refused its test socket).
This resolves my formatting change request. The existing security impact and independent maintainer/CI requirements remain applicable.
What
Enforce the RFC 8785 safe-integer ceiling (
2**53 - 1) acrossDelegationCredential,verify_chain,RevocationStatement, andRevocationSnapshot, and enforce unknown-field rejection and canonical lowercase hex onHolderProofandCallerPossession.Why
Closes #274.
Security impact
Prevents out-of-range integers (
> 2**53 - 1) from bypassingDelegationCredentialorRevocationSnapshotconstruction, prevents uppercase or whitespace-padded hex signatures inHolderProof, and prevents lone UTF-16 surrogates inCallerPossession.macfrom raising an unhandledUnicodeEncodeErrorinverify_caller_possession.Test plan
pytestpasses (pytest tests/unit/test_holder_binding.py tests/unit/test_caller_offer_relay.py tests/unit/test_delegation.py tests/unit/test_revocation.py -q:146 passed)ruff checkpasses (ruff check src/ca2a_runtime/delegation/credential.py src/ca2a_runtime/delegation/revocation.py src/ca2a_runtime/delegation/holder.py src/ca2a_runtime/attestation.py tests/unit/test_holder_binding.py:All checks passed!)mypypasses (mypy --strict src/ca2a_runtime/delegation/credential.py src/ca2a_runtime/delegation/revocation.py src/ca2a_runtime/delegation/holder.py src/ca2a_runtime/attestation.py:Success)DCO sign-off
Developer Certificate of Origin (https://developercertificate.org).