Refs #576: Reject control chars in treasury proposals

[Squirbie]

Summary

• Reject raw C0/DEL/C1 control characters in treasury proposal action and payload-string fields before trimming/canonicalization.
• Reject raw control characters in treasury proposal integer-string fields before parsing.
• Add regression coverage for proposal creation and challenge creation so malformed action, issue number, title, challenge type, and reason text fail before persistence.

Evidence

Bounty #576 is open. Before this change, a request such as action="\tcreate_bounty" to /api/v1/treasury/proposals was accepted as clean create_bounty and created a pending proposal. The same raw-control-character-before-trim pattern also existed for treasury proposal payload integers/strings and challenge strings.

This is distinct from the existing wallet/account/bounty/MCP/admin webhook/JSON integer/MRWK amount/wallet hex/activity control-character PRs because it covers treasury proposal and challenge canonicalization in app/treasury.py.

Validation

• uv run --extra dev python -m pytest tests/test_treasury_proposals.py::test_treasury_proposal_creation_rejects_raw_control_characters tests/test_treasury_proposals.py::test_treasury_challenges_reject_raw_control_characters -q -> 5 passed, 1 warning
• uv run --extra dev python -m pytest tests/test_treasury_proposals.py -q -> 28 passed, 1 warning
• uv run --extra dev python -m pytest -q -> 487 passed, 1 warning
• uv run --extra dev ruff check . -> passed
• uv run --extra dev ruff format --check app/treasury.py tests/test_treasury_proposals.py -> passed
• uv run --extra dev mypy app/treasury.py app/treasury_routes.py -> success
• uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
• git diff --check -> clean
• git merge-tree $(git merge-base HEAD origin/main) HEAD origin/main -> clean

No private data, wallet material, tokens, signatures, admin token values, production mutation, price/liquidity/exchange/bridge/off-ramp claims, or private security details are included.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Treasury proposal and challenge submission endpoints now validate input fields and reject requests containing control characters, returning appropriate error responses.

• Tests

  • Added parametrized test coverage to verify that treasury endpoints properly reject submissions with control characters and maintain data integrity.

[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: 9ff42c98-d573-48e9-a348-0abc94aadb2e

📥 Commits

Reviewing files that changed from the base of the PR and between e00dc75 and c85285a.

📒 Files selected for processing (2)

• app/treasury.py
• tests/test_treasury_proposals.py

---

📝 Walkthrough

Walkthrough

This PR hardens treasury endpoints against control character injection by introducing a shared control character validator and applying it to request parsing functions. It adds comprehensive parametrized tests confirming both proposal and challenge creation endpoints reject control characters in key fields.

Changes

Treasury Control Character Validation

Layer / File(s)	Summary
Control character validation helpers app/treasury.py	New _contains_control_character() helper detects ASCII and C1 control characters. Existing _clean_string() and _payload_int() functions updated to use this helper for early rejection before string trimming or integer parsing.
Endpoint control character rejection tests tests/test_treasury_proposals.py	Parametrized tests for /api/v1/treasury/proposals (action, issue_number, title fields) and /api/v1/treasury/proposals/{id}/challenges (challenge_type, reason fields) verify HTTP 400 responses with exact detail messages and confirm no database rows are created when control characters are present.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly names the changed surface and main intent: rejecting control characters in treasury proposals, addressing the specific issue (#576).
Description check	✅ Passed	The description covers all required sections: summary of changes, evidence of the problem, validation results, and test coverage confirmation. All checklist items are addressed with validation data.
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 modifies only code files (app/treasury.py, tests/test_treasury_proposals.py) with no investment, price, cash-out, or private security claims.
Bounty Pr Focus	✅ Passed	PR Refs #576 correctly targets treasury.py and tests with control character validation, parametrized test coverage for all required fields, HTTP 400 responses, no DB persistence, and no scope creep.

_{✏️ 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.}

[yui-stingray]

Reviewed current head c85285a035cea3cb0645efdb983407c43dd2ea67 and did not find a blocker.

I checked the focused treasury diff in app/treasury.py and tests/test_treasury_proposals.py. The shared control-character guard rejects C0, DEL, and C1 characters before trimming, and the integer payload path also rejects raw control characters before parsing. The added coverage exercises proposal action, issue_number, title, challenge challenge_type, and reason, with HTTP 400 responses and no persisted proposal/challenge rows on invalid input.

Validation performed:

• git diff --check origin/main...origin/pr-621 passed
• git merge-tree --write-tree origin/main origin/pr-621 succeeded
• tests/test_treasury_proposals.py passed: 28 tests
• py_compile on touched files passed
• Ruff check and format check on touched files passed
• docs smoke passed

CodeRabbit reported no actionable comments, and GitHub quality checks are green.

[eliasx45]

Reviewed current head c85285a035cea3cb0645efdb983407c43dd2ea67.

Verdict: approve from technical review.

The treasury proposal/challenge hardening is focused and behaves correctly in the slice I checked. _clean_string() now rejects raw C0, DEL, and C1 control characters before trimming, so values like tab-prefixed actions or C1-padded titles cannot normalize into valid proposal payload fields. _payload_int() also rejects raw control characters before trimming/parsing integer strings, so issue-number style payloads do not silently normalize. The new tests cover proposal action, issue_number, title, and challenge challenge_type/reason, and assert no proposal/challenge rows are persisted on the malformed paths.

Validation on an updated origin/main:

git diff --check origin/main...HEAD
# clean

git merge-tree --write-tree origin/main HEAD
# 57dc3ebf1c952c34bf4cd709996f094f19ac58f6

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest -q tests/test_treasury_proposals.py --tb=short
# 28 passed

python -m py_compile app/treasury.py tests/test_treasury_proposals.py
# passed

python -m ruff check app/treasury.py tests/test_treasury_proposals.py
# passed

python -m ruff format --check app/treasury.py tests/test_treasury_proposals.py
# 2 files already formatted

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python scripts/docs_smoke.py
# docs smoke ok

Note: this is a code review only. I am not claiming this under #578 because the maintainer has already posted that the current review-bounty round is at capacity.