fix(agent-mesh): make MerkleAuditChain root reproducible by an independent rebuild

[chopmob-cloud]

Summary

MerkleAuditChain in agent-mesh/src/agentmesh/governance/audit.py builds its tamper-evidence Merkle tree two ways that are meant to agree but do not, so the exported merkle_root is not reproducible by an independent verifier for most chain sizes.

Root cause

The class has two constructions:

• add_entry updates the tree incrementally. When it grows capacity it pads interior tree levels with singleton zero nodes (MerkleNode(hash="0" * 64)).
• _rebuild_tree (docstring: "full rebuild, used for verification") pads at the leaf level and hashes the zero padding upward, so an interior empty subtree is sha256(ZERO + ZERO)..., not "0" * 64.

The two roots agree while every interior node on a real leaf's authentication path is itself real, but diverge as soon as that path reaches an interior padding node. That first happens at five entries, and again at 6, 9, 10 and most non power of two sizes.

AuditLog.export() emits the incremental root as merkle_root. A third party (or the module's own _rebuild_tree) that recomputes a standard Merkle tree from the exported entries gets a different root and rejects a genuine, untampered log. This is a correctness and reproducibility defect in the integrity tooling; it does not let an attacker conceal tampering (the effect is over rejection of honest logs, and the live verify_integrity path uses the linear previous_hash chain, which is unaffected).

Reproduction

On a clean install of the shipped package, comparing get_root_hash() against an independent textbook Merkle recomputation of the same leaf hashes:

external-reproducibility n=1..32: FAIL at [5, 6, 9, 10, 11, 12, 13, 14, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30]

Fix

Rebuild the tree from all leaves on each add_entry via the existing _rebuild_tree, removing the divergent incremental interior padding. The recorded root is then byte identical to an independent recomputation for every entry count. A full rebuild is O(n) per append, which is negligible for audit volumes next to a non reproducible tamper evidence root.

Validation

Clean python:3.12-slim container, package installed from source.

• Before the fix: the two new reproducibility tests fail and the external recompute fails at 22 of the first 32 sizes (the bug, reproduced against the shipped artifact).
• After the fix: 49 passed across test_step_receipt_to_audit_chain.py, test_governance.py, and the new test_merkle_audit_root_reproducible.py, with the 45 pre existing audit tests unchanged (zero regressions).
• External recompute n=1..32: all reproduce; tampering with any leaf still changes the recomputed root.

New tests in test_merkle_audit_root_reproducible.py cover incremental vs rebuild agreement across the first two capacity doublings, the five entry minimal reproducer, inclusion proof verification against the recorded root, and tamper detection.

Note on classification

I read this as a correctness bug rather than an exploitable vulnerability, since it causes honest logs to fail verification rather than letting tampering pass, and no trust boundary is crossed. If maintainers see a security angle I have missed, I am happy to move the discussion to a private channel.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

🤖 AI Agent: docs-sync-checker — Docs Sync

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Docs Sync

Documentation is in sync.

[github-actions]

🤖 AI Agent: contributor-guide — What you did well:

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Welcome, and thank you for your detailed contribution! It's great to see the thorough analysis and testing you've included.

What you did well:

Your fix is well-documented, with clear reasoning and robust test coverage for reproducibility and tamper detection.

Actionable items:

1. The new test file test_merkle_audit_root_reproducible.py directly calls the private method _rebuild_tree. Consider refactoring to avoid testing private methods directly, as this may lead to brittle tests.

For more details on contributing, please refer to our CONTRIBUTING.md.

[github-actions]

🤖 AI Agent: breaking-change-detector — API Compatibility

AI-generated review output. Treat it as untrusted analysis and verify before acting.

API Compatibility

Severity	Change	Impact
High	MerkleAuditChain.add_entry now uses a canonical empty-subtree constant (E(k)) for interior padding instead of a fixed zero-leaf sentinel ('0' * 64).	Exported merkle_root values for affected chain sizes (e.g., 5, 6, 9-14, 17-30) will differ from previous versions, potentially causing verification mismatches for previously archived logs.
High	MerkleAuditChain.get_root_hash now returns a canonical, reproducible Merkle root.	Consumers relying on the previous non-canonical root for verification will need to update their processes to align with the new behavior.
High	Inclusion proofs from MerkleAuditChain.get_proof are now based on the canonical Merkle root.	Previously generated inclusion proofs may no longer validate against the updated root, requiring regeneration of proofs for affected chain sizes.
High	AuditLog.export (merkle_root / chain_root fields) now reflects the canonical root.	Archived logs with the old root format will mismatch when verified against the updated implementation.

[github-actions]

🤖 AI Agent: code-reviewer — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

TL;DR: 0 blockers, 1 warning. The fix improves Merkle tree reproducibility but introduces a breaking change requiring downstream consumers to re-verify archived logs.

#	Sev	Issue	Where
1	Warn	Breaking change: merkle_root values for certain chain sizes will differ from prior versions.	agent-mesh/src/agentmesh/governance/audit.py, BREAKING_CHANGES.md, CHANGELOG.md

Action items:

1. None.

Warnings:

#	Description	Resolution
1	Breaking change in merkle_root values requires downstream consumers to re-verify archived logs.	Fine as follow-up PRs: Ensure affected users are notified and provide migration guidance.

[github-actions]

🤖 AI Agent: test-generator — `agentmesh/governance/audit.py`

AI-generated review output. Treat it as untrusted analysis and verify before acting.

agentmesh/governance/audit.py

• test_add_entry_interior_padding -- Validate that interior padding uses _empty_subtree_hash consistently.
• test_empty_subtree_hash_growth -- Test lazy growth of _empty_hashes to ensure correct padding constants.
• test_add_entry_root_update -- Verify that add_entry correctly updates the root hash after each addition.
• test_rebuild_tree_consistency -- Ensure _rebuild_tree produces the same root as incremental updates for all sizes.

test_merkle_audit_root_reproducible.py

• test_tamper_detection -- Validate that tampering with any leaf changes the recomputed root.
• test_inclusion_proof_verification -- Test inclusion proof verification against the recorded root.

[github-actions]

🤖 AI Agent: security-scanner — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

No security issues found.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

[github-actions]

🔴 Contributor Check: HIGH

Check	Result
Profile	HIGH
Credential	LOW
Overall	HIGH

Automated check by AGT Contributor Check.

[MohammadHaroonAbuomar]

• Replace full-rebuild-per-append with a root-equivalent incremental fix. Measured: main appends 5000 entries in 0.24s; this PR takes 87.6s (~365x, O(n^2)). ADR-0017 requires sub-millisecond per-entry overhead and AUDIT-COMPLIANCE-1.0.md section 9.3 says the tree MUST be built incrementally. Preferred ~5-line fix: pad interior level k with the empty-subtree constant E(k) where E(0)='0'*64, E(k+1)=SHA-256(E(k)||E(k)) — both constructions then converge to the canonical root with O(log n) appends. Alternative: dirty-flag lazy rebuild in get_root_hash/get_proof.
• test_incremental_root_matches_rebuild_for_every_size is tautological after the fix: _rebuilt_root() calls chain._rebuild_tree(), but add_entry now IS _rebuild_tree, so the test compares the function with itself and can never fail. Inline a genuinely independent pure-hashlib textbook recomputation instead. Also add the empty-chain case (root is None).
• Add a BREAKING_CHANGES.md entry: exported merkle_root values change for most chain sizes (22 of the first 32) vs prior releases; consumers who archived export() output for compliance evidence will see mismatches on re-verification.
• (follow-up, non-blocking) pin the canonical construction in AUDIT-COMPLIANCE-1.0.md 9.3; consider RFC-6962-style leaf/interior domain separation while roots are already changing; delete the now-dead odd-duplication branch in _rebuild_tree.

Minor:

• The bug itself is real and precisely diagnosed — verified empirically: main diverges from an independent textbook recomputation at n=5,6,9-14,17-30; the fix diverges nowhere. Severity honestly classified (over-rejection of honest logs, not tamper concealment).

[chopmob-cloud]

Thanks, this is exactly right on all points. Pushed the preferred incremental fix.

E(k) incremental (no more full rebuild). add_entry now pads interior level k with the empty-subtree constant E(k) (E(0) = '0' * 64, E(k+1) = SHA-256(E(k) || E(k))), memoized on the instance, instead of the leaf sentinel at every level. The incremental construction converges on the same canonical root as _rebuild_tree, and the update is back to O(log n) per append. On the CI-parity box, 5000 appends take ~0.57s (~0.11 ms/entry), well under the ADR-0017 sub-millisecond bar, versus the ~88s you measured for the rebuild version.

Tests are no longer tautological. The reproducibility test now recomputes the root with an independent pure-hashlib textbook Merkle that shares no code with MerkleAuditChain (I inlined the same construction I used to verify the bug). Added the empty-chain case (get_root_hash() is None) and a separate incremental == _rebuild_tree cross-check so both paths are pinned.

Docs. Added a BREAKING_CHANGES.md entry and an agent-mesh/CHANGELOG.md note: exported merkle_root values change for the affected sizes (22 of the first 32) versus prior releases, so consumers who archived export() output for compliance evidence will see a mismatch on re-verification; entry hashes and the linear previous_hash chain are unchanged.

Also done. Removed the now-dead odd-duplication branch in _rebuild_tree (leaves are padded to a power of two, so every level is even).

Verification: incremental == independent-textbook == _rebuild_tree for n = 1..64, and the audit test suite passes 51/51 on a clean python:3.12 box.

On the non-blocking follow-ups: happy to pin the canonical construction in AUDIT-COMPLIANCE-1.0.md 9.3 and to add RFC-6962-style leaf/interior domain separation while roots are already changing, if you would like those in this PR or a follow-up. Thanks again for the careful review and the independent reproduction.

[chopmob-cloud]

Thanks for the review, and for pinning the divergence down to the exact sizes. All four points are addressed on the branch (commit a438ab4); each maps to a change below.

1. O(n^2) rebuild-per-append to E(k) incremental padding. add_entry now pads each interior level with its own empty-subtree constant via _empty_subtree_hash(level), where E(0) = '0'*64 and E(k+1) = SHA-256(E(k) || E(k)), the construction you preferred. The incremental root converges on the canonical from-scratch root, and each append is back to O(log n) work rather than a full rebuild.

2. Tautological test to independent recomputation. tests/test_merkle_audit_root_reproducible.py adds _textbook_merkle_root(), a pure-hashlib textbook root that shares no code with MerkleAuditChain, and asserts the incremental root matches it for every size 1..32 (covering 5, 6, 9..14, 17..30). The empty-chain case is covered by test_empty_chain_root_is_none.

3. BREAKING_CHANGES.md added, noting exported merkle_root values change for most chain sizes and that archived export() evidence will mismatch on re-verification.

4. (non-blocking) The dead odd-duplication branch in _rebuild_tree is removed; leaves are padded to a power of two, so every node has a right sibling.

CI is green. The RFC-6962 leaf/interior domain separation and the AUDIT-COMPLIANCE 9.3 pinning I have left as the follow-ups you flagged, to keep this PR to the root-reproducibility fix. Re-requesting review when you have a moment.