Refs #576: Reject control chars in webhook identities

[Squirbie]

Summary

• reject raw C0, DEL, and C1 control characters in accepted-label webhook repository, submitter, and sender identity fields before trimming
• record malformed repository payloads before bounty lookup and keep malformed submitter/sender payloads from reaching payout or accepted-labeler checks
• add signed webhook regression coverage for tab-padded repo/submitter and C1-padded sender values

Validation

• uv run --extra dev python -m pytest tests/test_webhooks.py::test_accepted_label_rejects_control_characters_before_trimming_webhook_identity -q
• uv run --extra dev python -m pytest tests/test_webhooks.py -q
• uv run --extra dev python -m pytest tests/test_security.py -q
• uv run --extra dev python -m pytest -q
• uv run --extra dev ruff check app/webhooks/github.py tests/test_webhooks.py
• uv run --extra dev ruff format --check app/webhooks/github.py tests/test_webhooks.py
• uv run --extra dev mypy app/webhooks/github.py
• uv run --extra dev python scripts/docs_smoke.py
• git diff --check
• git merge-tree $(git merge-base HEAD origin/main) HEAD origin/main | wc -c -> 0

/claim #576

Summary by CodeRabbit

• Bug Fixes

  • Enhanced webhook payload validation to detect and reject fields containing control characters.

• Tests

  • Added test coverage for webhook payload validation with malformed identity data.

[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: f93ed0d9-95ed-4225-b596-54d34191bc32

📥 Commits

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

📒 Files selected for processing (2)

• app/webhooks/github.py
• tests/test_webhooks.py

---

📝 Walkthrough

Walkthrough

Added control-character detection to GitHub webhook handling. The webhook identity fields (repository name, submitter login, sender login) are now validated to reject control characters before processing; if fields contain control characters or are missing, specific rejection statuses (malformed_repository, missing_submitter, missing_sender) are recorded instead of proceeding with unsafe values.

Changes

Webhook Identity Field Validation

Layer / File(s)	Summary
Control-character detection utility app/webhooks/github.py	Added compiled regex pattern (CONTROL_CHAR_RE) to detect control characters and a _clean_webhook_string(value) function that returns a stripped string only if the input contains no control characters, otherwise returns None.
Webhook handler identity field validation app/webhooks/github.py	Updated _handle_accepted_issue_label to use _clean_webhook_string for repository name, submitter login, and sender login validation; records specific malformed/missing statuses when fields contain control characters or are invalid instead of proceeding with unsafe values.
Control-character rejection test tests/test_webhooks.py	Added test that verifies webhook payloads containing control characters in identity fields are rejected before trimming; iterates over malformed variations and asserts both handler return status and persisted WebhookEvent.processed_status for each delivery.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the specific change: rejecting control characters in webhook identity fields, which matches the core objective of the pull request.
Description check	✅ Passed	The description covers the main change, test coverage, validation steps, and references issue #576, though some template sections like Evidence are condensed rather than fully expanded.
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	Technical security patch with no problematic claims. MRWK correctly described as native ledger coin in docs.
Bounty Pr Focus	✅ Passed	PR adds control-char detection to webhook identity fields (repository, submitter, sender) with proper rejection before bounty lookup. Test coverage includes tab and C1 chars. Scope focused on #576.

_{✏️ 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 c649dcecdacf18bf7b66a561e1cba63ff63915b3 and did not find a blocker.

I checked the focused webhook diff in app/webhooks/github.py and tests/test_webhooks.py. The new control-character regex covers C0, DEL, and C1 characters, and _clean_webhook_string() rejects those values before trimming. The accepted-label handler now rejects malformed repository identity data and missing/invalid submitter or sender identity before bounty lookup or payout handling. The regression covers repository, submitter, and sender identity inputs and confirms no balance is credited when those malformed deliveries are processed.

Validation performed:

• git diff --check origin/main...origin/pr-622 passed
• git merge-tree --write-tree origin/main origin/pr-622 succeeded
• tests/test_webhooks.py passed: 22 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 c649dcecdacf18bf7b66a561e1cba63ff63915b3.

Verdict: approve from technical review.

The webhook identity hardening is focused on the accepted-label payout path. _clean_webhook_string() rejects raw C0, DEL, and C1 control characters before trimming, and the accepted-label handler now applies it to repository full name, submitter login, and sender login before bounty lookup or payout resolution. The regression covers malformed repository, submitter, and sender identity values, verifies the expected non-credit statuses, and confirms the claimant balance is not credited for malformed deliveries.

Validation on an updated origin/main:

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

git merge-tree --write-tree origin/main HEAD
# 63aaa17c5f5f01cc7db9bbebe896eca90da35554

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

python -m py_compile app/webhooks/github.py tests/test_webhooks.py
# passed

python -m ruff check app/webhooks/github.py tests/test_webhooks.py
# passed

python -m ruff format --check app/webhooks/github.py tests/test_webhooks.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.