[drk-ram]

What

Enforce canonical unpadded base64url in cmcp_runtime.policy.bundle._b64url_decode and cmcp_runtime.catalog.approval._decode, require strict ASCII decimal segments in parse_bundle_version, and reject duplicate JSON keys and NaN/Infinity in policy bundle manifests and revocation files.

Why

Refs #802. The static bundled _approval_schema JSON loader remains tracked there as a separate follow-up.

Security impact

Prevents malleable Ed25519 signatures (non-zero unused low padding bits) in policy bundles, key revocations, and catalog-approval records, prevents negative or non-ASCII integers in parse_bundle_version, and blocks duplicate-key JSON smuggling in manifest.json and signing-key-revocations.json.

Test plan

• pytest passes (pytest tests/unit/test_policy_bundle.py tests/unit/test_policy_signing.py tests/unit/test_policy_key_revocation.py tests/unit/test_catalog_approval.py -q)
• ruff check passes (ruff check src/cmcp_runtime/policy/bundle.py src/cmcp_runtime/catalog/approval.py tests/unit/test_policy_bundle.py)
• mypy passes (mypy src/cmcp_runtime/policy/bundle.py src/cmcp_runtime/catalog/approval.py)
• Manual test performed

DCO sign-off

• I certify that I wrote or have the right to submit this contribution, and I agree to the
  Developer Certificate of Origin (https://developercertificate.org).

[github-actions]

🟡 Contributor Check: MEDIUM

Check	Result
Profile	MEDIUM
Credential	LOW
Overall	MEDIUM

Automated check by AgenTrust Contributor Check.

[carloshvp]

Reviewed exact head 0e42f829620840e6bf79898f393f87e25b8f7bbc in an isolated checkout.

Requesting changes for a bounded validation finding in the changed security-sensitive path. I submitted the reproducer and a comparison against the actual merge-base behavior through the repository's private vulnerability-reporting channel, following SECURITY.md. Maintainers can find report GHSA-w5v5-4fgp-26x2; it is in triage and unpublished. Please address that report before this head is approved. This is a pre-merge regression report; I have not established released-version impact or additional attacker capability.

Separately, the ordinary local checks pass: 2,532 unit tests passed, 20 skipped in a hash-locked Python 3.12.10 environment; pip check, Ruff, mypy and Bandit passed. All commits have sign-offs. Those passing checks do not cover the privately reported control. git diff --check also flags an extra blank line at test_policy_bundle.py EOF.

Reviewed with OpenAI Codex.

[noah-ing]

Reviewed exact head 0e42f829620840e6bf79898f393f87e25b8f7bbc. My independent signed before/after replay confirms the validation regression already reported to maintainers through the private security channel. Requesting changes until that finding is fixed and covered by a targeted regression test; please coordinate the reproducer privately with the maintainers. I am keeping its mechanics out of this public review.

The ordinary exact-head unit suite passed (2,532 passed, 20 skipped), but it does not exercise the reported case. I saw no substantive hosted CI result for this head; the maintainer gate remains outstanding. This review does not assert released-version impact.

[drk-ram]

Thanks @carloshvp and @noah-ing! Fixed the pre-merge regression in PolicySigningKeys.apply_file (src/cmcp_runtime/policy/bundle.py): a statement in signing-key-revocations.json containing duplicate JSON keys or NaN/Infinity now marks only that statement with _INVALID_REVOCATION_JSON (rejected individually in PolicySigningKeys.apply with PolicyKeyRevocationInvalid) rather than aborting json.loads() for the entire revocation file and blocking valid signed revocation statements in the same file. Added test_revocation_file_rejects_duplicate_key_statement_without_blocking_valid_revocation in tests/unit/test_policy_bundle.py and trimmed the EOF blank line.

[noah-ing]

Thanks for the update. I reviewed exact head 5e9a8c00ba4d0ccc3ecb0e0dd0578b9835208a96. The original signed mixed-statement regression is fixed: the later valid revocation now applies, and the new committed test distinguishes the merge base from the previous PR head. Focused tests passed (58/58); the full local unit suite passed (2,533 passed, 20 skipped).

One narrow strict-JSON case remains. _revocation_object_pairs propagates invalid markers through a list only one level deep. A valid signed statement with an ignored extra field containing nested arrays around a duplicate-key object or NaN is accepted by apply_file, although #802 calls for duplicate/non-finite rejection in revocation JSON. I found no authority escalation or suppression of another valid revocation through this case. Please reject invalid markers recursively through nested arrays and add signed negative controls. Ruff also fails I001 on the new import block at tests/unit/test_policy_bundle.py:451. I am keeping the changes request open for these focused corrections. Hosted substantive CI has not run on this head.

[dinakarjs]

I prepared a signed adoption companion addressing the outstanding review finding: dinakarjs#7

Validation: source repair unit suite 2547 passed/6 skipped; conformance+integration+confinement 114 passed/23 skipped. Final test-only parameterization rerun: all 37 policy-bundle tests pass. Its four nested-array cases each fail by assertion when the source repair is removed, with test names retained. Ruff src/tests, strict mypy and Bandit passed. No claim of authorization escalation: this closes strict-JSON acceptance.

Please cherry-pick/adopt the companion and request independent re-review of the resulting head. I have contributed the follow-up and am not approving my own contribution.

[drk-ram]

Thanks @noah-ing! Added _contains_invalid_revocation to walk nested arrays recursively in _revocation_object_pairs, added signed negative controls for nested arrays around duplicate-key objects and NaN in tests/unit/test_policy_bundle.py, and fixed the I001 import sorting.

[drk-ram]

Thanks @dinakarjs and @noah-ing! Adopted @dinakarjs's signed companion commit 6546592 (fix/review-807-20261025) directly as the branch head with authorship and signature preserved.

[noah-ing]

Reviewed exact head 654659216ece77f4e47bc78eb98aca703b8849f4. The signed revocation-row regression and nested invalid-JSON case from my prior review now behave correctly in independent before/after controls: an invalid row is rejected without suppressing the later valid signed row. The focused policy-bundle tests passed (37/37); the full unit suite passed (2,536 passed, 20 skipped), and Ruff, changed-source mypy, dependency and whitespace checks passed. This resolves my requested changes, so I approve this scoped patch.

Nonblocking issue tracking: #802’s summary also names _approval_schema, whose static bundled schema loader still uses permissive JSON parsing. I found no authority effect from that path. Please track it separately or adjust the Closes #802 reference before merge so it is not lost when the issue closes. Substantive hosted CI still needs to run under the normal gate.

[carloshvp]

Reviewed exact head 654659216ece77f4e47bc78eb98aca703b8849f4 in an isolated checkout.

My previous pre-merge validation finding is addressed by the adopted repair at this head. I independently replayed the privately reported case against both the previously reviewed source and the current source, with valid signed controls and rejection controls. The current implementation passes those controls while preserving rejection of an ambiguous signed statement. Details of the originally reported case remain in unpublished report GHSA-w5v5-4fgp-26x2; this public review omits the reproducer.

In a fresh hash-locked Python 3.12.10 environment, the full unit suite passed: 2,536 passed, 20 skipped. pip check, Ruff, mypy and configured Bandit also passed; the unit run used the loopback permissions needed by its local transport fixtures. Commit sign-offs are present. This supersedes my earlier code changes request at the older head. The private report remains unpublished/in triage, and required security-maintainer approvals, hosted CI and any advisory/release disposition remain separate. No released-version impact is asserted by this review.

Reviewed with OpenAI Codex.

[carloshvp]

Private-report follow-up at unchanged head 654659216ece77f4e47bc78eb98aca703b8849f4: I reran the original paired control against this head and the previously reported source. The adopted repair still passes; the older source still fails the reported control. This agrees with the existing independent repair reviews. Reproduction details remain in GHSA-w5v5-4fgp-26x2.

The report remains in triage because GitHub returned HTTP 403 when I tried to record its resolved pre-merge disposition: this account lacks the security-advisory triage permission. An authorized repository security triager should disposition that private report using the repair evidence. This is not a claim about a released affected version. The PR still needs its normal merge/CI gates.

I also changed the description to Refs #802, preserving the issue for the separately noted static _approval_schema loader follow-up. No source or reviewer approval was changed.

Reviewed with OpenAI Codex.

[drk-ram]

@carloshvp @noah-ing @dinakarjs Rebased onto origin/main (1fbc1b2) in 39fea26 (resolving the tests/unit/test_policy_bundle.py conflict with #785 and picking up the pip-audit fix) and added UTF-8 / duplicate-key / non-finite validation to _approval_schema() in src/cmcp_runtime/catalog/approval.py.

[noah-ing]

Reviewed current head 39fea2606d37d98bfa8fc5de3a16544c7b6c67df. The rebased patch and new approval-schema parser pass 76 focused policy/approval tests in a fresh Python 3.12 environment. One strict JSON boundary remains incomplete.

PolicySigningKeys.apply_file (src/cmcp_runtime/policy/bundle.py, around line 284) and _approval_schema (src/cmcp_runtime/catalog/approval.py, lines 183–187) reject literal NaN/Infinity via parse_constant, but Python's JSON parser converts exponent overflow such as 1e999 to infinity through its normal float path. In paired synthetic controls, a validly signed revocation with an otherwise ignored extra: 1e999 was applied and promoted the successor key; the same statement with extra: NaN was rejected, and an invalid signature was rejected. A schema containing an ignored 1e999 field also loaded. The extra value is ignored after parsing, and I found no signature bypass or authorization escalation. The issue is that the stated non-finite input rejection does not cover this route.

Please enforce finite parsed floats at both JSON loaders and add top-level/nested overflow regressions alongside finite controls, including a check that a rejected revocation leaves key state unchanged. I request changes on this bounded input-contract point. The hosted maintainer gate and independent advisory disposition remain separate.

[drk-ram]

@noah-ing Added finite parse_float hooks (math.isfinite) to PolicySigningKeys.apply_file, load_policy_bundle (src/cmcp_runtime/policy/bundle.py), and _approval_schema (src/cmcp_runtime/catalog/approval.py) in 14b14e2, along with top-level and nested exponent-overflow (1e999, -1e999) and finite (1.5) controls in tests/unit/test_policy_bundle.py (81 passed).