Refs #406: Guard nested proof metadata control chars

[Baijack-star]

Summary

• recursively reject control characters in verifier_result strings before public proof JSON is persisted
• reject control characters in nested verifier_result object keys without echoing the unsafe key text
• cover nested dict/list rejection and safe nested metadata preservation

Evidence

• Confusion/bug addressed: the existing proof metadata cleaner only checked top-level string values, so nested strings or nested object keys containing control characters could enter public proof JSON.
• Bounty status checked: MRWK bounty: useful bug reports and small fixes, round 5 #406 is open and has awards remaining at the time of this work.
• Intended files: app/ledger/service.py and tests/test_security.py.
• Expected PR size: small service helper plus focused security regressions.
• Out of scope: no payout execution, wallet handling, settlement setup, API schema change, or public price/exchange/bridge claims.

Validation

• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_security.py::test_bounty_payment_proof_rejects_control_character_metadata tests/test_security.py::test_bounty_payment_proof_rejects_nested_control_character_metadata tests/test_security.py::test_bounty_payment_proof_preserves_safe_nested_metadata -q -> 5 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_security.py tests/test_ledger.py -q -> 74 passed
• ./.venv/bin/python -m ruff check app/ledger/service.py tests/test_security.py -> passed
• ./.venv/bin/python -m ruff format --check app/ledger/service.py tests/test_security.py -> 2 files already formatted
• ./.venv/bin/python -m mypy app/ledger/service.py -> success
• git diff --check -> clean

Refs #406

Summary by CodeRabbit

• Bug Fixes

  • Strengthened validation of proof/verifier metadata to reject control characters at any nesting level and to fail safely on excessively nested or self-referential structures.

• Tests

  • Added security tests covering nested metadata sanitization, rejection of recursive/self-referencing payloads with no payment or persisted records, and preservation of safe nested metadata with successful payout.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4f8f2831-c8e6-4cbd-a074-30cd85a8ea3e

📥 Commits

Reviewing files that changed from the base of the PR and between 7a31ed8 and c0e8937.

📒 Files selected for processing (2)

• app/ledger/service.py
• tests/test_security.py

---

📝 Walkthrough

Hidden review stack artifact

Walkthrough

This PR extends proof metadata control-character validation from checking only top-level string values to recursively validating nested dict keys/values and list elements. A new helper function implements the recursive traversal, and two security regression tests confirm rejection of malicious nested structures and preservation of safe metadata.

Changes

Metadata Control-Character Validation

Layer / File(s)	Summary
Recursive metadata control-character validation app/ledger/service.py	New _reject_metadata_control_chars helper recursively traverses nested dict and list structures to detect and reject control characters in string values and object keys; _clean_proof_metadata now delegates validation to this recursive helper and converts RecursionError into a LedgerError.
Security regression tests for nested metadata validation tests/test_security.py	Parametrized test verifies pay_bounty rejects control characters in nested verifier_result structures (nested dict values, list elements, object keys), asserting LedgerError and no awards/records. Additional tests assert recursive self-references are rejected with a bounded-depth LedgerError, and that safe nested metadata is preserved verbatim in returned proof JSON and stored submission verifier data with payment confirmed.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly names the concrete change: recursive validation of nested proof metadata control characters, clearly mapping to the main alteration in app/ledger/service.py.
Description check	✅ Passed	The description includes all required sections: Summary, Evidence (addressing confusion, bounty status, intended files, scope), validation checklist (all items checked), and issue reference.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Mergework Public Artifact Hygiene	✅ Passed	PR contains no prohibited investment/price/cash-out claims or private security details. MRWK correctly described as native coin in README per AGENTS.md hygiene guidelines.
Bounty Pr Focus	✅ Passed	PR targets only stated files, adds recursive control-char validation with RecursionError handling, and includes three security tests for nested metadata and safe preservation as specified.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0c87e17c-1a08-482c-a748-20a964bbcbe6

📥 Commits

Reviewing files that changed from the base of the PR and between d8532d4 and 7a31ed8.

📒 Files selected for processing (2)

• app/ledger/service.py
• tests/test_security.py

[tinyopsstudio]

Reviewed PR #551 at 7a31ed8e8dc77cbcc6c29e69750b6e9aa0e8240f for nested verifier metadata control-character handling.

Evidence checked:

• inspected app/ledger/service.py and tests/test_security.py;
• confirmed _clean_proof_metadata() now runs a recursive control-character pass before canonical_json() and still rejects non-serializable verifier metadata through the existing JSON guard;
• confirmed nested dict values, list values, and nested object keys with control characters are rejected before public proof JSON or submissions are persisted;
• confirmed safe nested metadata remains preserved in both Proof.public_json and Submission.verifier_result.

Validation:

• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_security.py::test_bounty_payment_proof_rejects_control_character_metadata tests/test_security.py::test_bounty_payment_proof_rejects_nested_control_character_metadata tests/test_security.py::test_bounty_payment_proof_preserves_safe_nested_metadata -q -> 5 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_security.py tests/test_ledger.py -q -> 74 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest -q -> 418 passed
• uv run --extra dev ruff check app/ledger/service.py tests/test_security.py -> passed
• uv run --extra dev ruff format --check app/ledger/service.py tests/test_security.py -> 2 files already formatted
• git diff --check origin/main...HEAD -> clean

Assessment: no blocker found. The patch is narrowly scoped to proof metadata cleanup and adds regression coverage for the public-proof persistence boundary.

[Baijack-star]

Follow-up for CodeRabbit recursion concern:

• Moved recursive verifier metadata validation into the bounded metadata-cleaning try block.
• Convert RecursionError into LedgerError("verifier_result is too deeply nested") so cyclic/deep metadata cannot surface as an unhandled server error.
• Added test_bounty_payment_proof_rejects_recursive_metadata to cover a self-referential verifier_result and assert no award, submission, proof, or balance change is created.

Validation after the follow-up commit c0e8937:

• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_security.py::test_bounty_payment_proof_rejects_nested_control_character_metadata tests/test_security.py::test_bounty_payment_proof_rejects_recursive_metadata tests/test_security.py::test_bounty_payment_proof_preserves_safe_nested_metadata -q -> 5 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_security.py tests/test_ledger.py -q -> 75 passed
• ./.venv/bin/python -m ruff check app/ledger/service.py tests/test_security.py -> passed
• ./.venv/bin/python -m ruff format --check app/ledger/service.py tests/test_security.py -> passed
• ./.venv/bin/python -m mypy app/ledger/service.py -> passed
• git diff --check -> clean

[Baijack-star]

Additional local validation after follow-up commit c0e8937: PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest -q -> 419 passed in 10.33s.

[eliasx45]

Reviewed current head c0e893726fa917b70768f40b593eb19ada50aa83 after the recursive/self-referential metadata follow-up.

No blocker found. The proof metadata cleaner now recursively rejects control characters in nested verifier-result values, list elements, and object keys before public proof JSON or submission verifier JSON is persisted. The follow-up also keeps that recursive pass inside the metadata-cleaning error boundary, so self-referential or too-deep verifier metadata is converted into a bounded LedgerError("verifier_result is too deeply nested") rather than escaping as an unhandled recursion failure. Safe nested metadata is still preserved in both Proof.public_json and Submission.verifier_result.

Validation run locally on this head:

• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .\.venv\Scripts\python.exe -m pytest tests\test_security.py::test_bounty_payment_proof_rejects_nested_control_character_metadata tests\test_security.py::test_bounty_payment_proof_rejects_recursive_metadata tests\test_security.py::test_bounty_payment_proof_preserves_safe_nested_metadata -q -> 5 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .\.venv\Scripts\python.exe -m pytest tests\test_security.py tests\test_ledger.py -q -> 75 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .\.venv\Scripts\python.exe -m pytest -q -> 419 passed
• .\.venv\Scripts\python.exe -m ruff check app\ledger\service.py tests\test_security.py -> passed
• .\.venv\Scripts\python.exe -m ruff format --check app\ledger\service.py tests\test_security.py -> 2 files already formatted
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .\.venv\Scripts\python.exe -m mypy app\ledger\service.py -> success
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .\.venv\Scripts\python.exe scripts\docs_smoke.py -> docs smoke ok
• git diff --check origin/main...HEAD -> clean

Hosted Quality/readiness and CodeRabbit are green. This looks mergeable from my reviewed slice.

[barnacleagent-svg]

Verdict: APPROVED

Scope: Guards nested proof metadata control chars with recursive _reject_metadata_control_chars function. Handles strings, dicts, lists recursively. Also adds RecursionError protection.

Checklist:

• Recursive validation covers all nesting levels
• Replaces flat iteration with proper recursive check
• Adds RecursionError catch for safety
• Tests cover nested scenarios

Conclusion: Well-implemented security hardening. Ready to merge.