Skip to content

fix(challenge,trace_binding): validate challenge secret/ttl/now/expiry and enforce strict iat/credential_id in trace_binding (#285) - #290

Open
drk-ram wants to merge 1 commit into
agentrust-io:mainfrom
drk-ram:drk-ram-fix-challenge-trace-285
Open

drk-ram wants to merge 1 commit into
agentrust-io:mainfrom
drk-ram:drk-ram-fix-challenge-trace-285

Conversation

@drk-ram

@drk-ram drk-ram commented Oct 25, 2026

Copy link
Copy Markdown

What

Validate secret (bytes of length >= 16), exact int ttl_seconds and now, and ASCII decimal expiry in ca2a_runtime.challenge, and validate iat (positive RFC 8785 safe int), subject, credential_id, parent_record_hash, and non-empty hops in ca2a_runtime.trace_binding.

Why

Closes #285.

Security impact

Prevents verify_challenge(..., now=False) from treating the current time as epoch 0 (making expired challenges appear unexpired), rejects empty/short HMAC challenge secrets, and prevents boolean/float/out-of-range iat or empty non-root credential_id in signed TRACE records.

Test plan

  • pytest passes (pytest tests/unit/test_challenge.py -q: 45 passed)
  • ruff check passes (ruff check src tests/unit/test_challenge.py: All checks passed!)
  • mypy passes
  • Manual test performed

DCO sign-off

@drk-ram
drk-ram requested review from a team, carloshvp and zohebk8s as code owners October 25, 2026 06:01
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:MEDIUM Contributor check flagged MEDIUM risk label Oct 25, 2026

@carloshvp carloshvp 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.

Reviewed exact head ce7721ff4a06a5a0500776fa7a2452be285eb80a in an isolated checkout.

[P2] Preserve the domain-error contract when converting a decimal expiry.

The new ASCII/decimal precheck removes the earlier try/except around int(expiry_str). With a valid secret and a syntactically shaped challenge whose expiry contains 4,301 ASCII decimal digits, Python 3.12's default integer-string limit raises ValueError at src/ca2a_runtime/challenge.py. verify_challenge documents AttestationFailed for a malformed/unissued challenge; the actual merge-base implementation returned that domain error for this same input. The new ValueError escapes instead.

Please keep the ASCII/decimal guard and translate a ValueError from int(expiry_str) back into AttestationFailed, or reject an excessive expiry length through that domain error before conversion. A focused in-process correction restoring the catch produces AttestationFailed for the oversized input while the valid issued-challenge control still passes. Add that regression alongside the non-decimal cases. This is a demonstrated error-contract regression; no authentication bypass is claimed.

The ordinary exact-head suite passes (800 passed, 16 skipped), as do pip check, Ruff, mypy, configured Bandit, sign-offs and diff whitespace checks. Requesting the narrow conversion-error repair before approval.

Reviewed with OpenAI Codex.

…y and enforce strict iat/credential_id in trace_binding (agentrust-io#285)

Validate secret bytes/length, ttl_seconds, now, and ASCII decimal expiry
(preserving AttestationFailed when int(expiry_str) exceeds the integer
string conversion limit) in ca2a_runtime.challenge, and enforce strict
integer iat and non-empty credential_id in ca2a_runtime.trace_binding.

Closes agentrust-io#285

Signed-off-by: Raghuvar Karthik Durga <177662415+drk-ram@users.noreply.github.com>
@drk-ram
drk-ram force-pushed the drk-ram-fix-challenge-trace-285 branch from ce7721f to d32fe0d Compare October 29, 2026 15:23
@drk-ram

drk-ram commented Oct 29, 2026

Copy link
Copy Markdown
Author

Thank you @carloshvp! Addressed in d32fe0d (rebased onto origin/main at 1889ce4):

  • Kept the expiry_str.isascii() and expiry_str.isdecimal() precheck and restored try: expiry = int(expiry_str) except ValueError as exc: raise AttestationFailed("challenge expiry is not an integer") from exc in src/ca2a_runtime/challenge.py.
  • Added the 4,301-digit decimal expiry case (f"v1.{'9' * 4301}.{'a' * 32}.{'0' * 64}") to test_verify_challenge_rejects_non_str_and_non_decimal_expiry in tests/unit/test_challenge.py.

@carloshvp carloshvp 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.

Reviewed current head d32fe0d00c90da939e20f239d10d95be8b845fb8 in an isolated checkout.

The oversized decimal-expiry conversion now translates the integer-conversion limit into AttestationFailed; the 4,301-digit regression reaches that path. The secret/TTL/time and TRACE binding validations remain intact. This resolves my earlier challenge-parser change request.

Fresh Python 3.12.10 validation: 857 passed, 16 skipped; Ruff, mypy and configured Bandit pass. The installed dependencies are consistent, and the PR commit carries a DCO sign-off. The required format check and whitespace check pass. Hosted CI and the required independent security approvals remain separate.

Reviewed with OpenAI Codex.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:MEDIUM Contributor check flagged MEDIUM risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

challenge / trace_binding: boolean ttl_seconds/now/iat bypasses, unvalidated challenge secret/expiry_str, and empty credential_id in emit_dag

2 participants