Reject ambiguous admin webhook event query params

[xtakenotesx16]

Summary

Tightens /api/v1/admin/webhook-events query parsing so it fails closed for the same ambiguous scalar inputs already rejected by nearby API routes.

Fixes the verified report for #656.

Changes

• Reject repeated status query parameters.
• Reject repeated limit query parameters.
• Reject non-canonical integer spellings for limit, such as 01.
• Preserve the existing clean admin webhook event filter path.

Validation

.\\.venv\\Scripts\\python.exe -m pytest tests\\test_bounty_api.py -q
19 passed, 1 warning

.\\.venv\\Scripts\\python.exe -m ruff format --check app tests
85 files already formatted

.\\.venv\\Scripts\\python.exe -m ruff check app tests
All checks passed!

git diff --check
passed

Bounty

This is a fix for the report submitted under MRWK bounty: 75 MRWK - live verification and bug reports, round 1.

Reference: #656

Summary by CodeRabbit

• Bug Fixes
  • Webhook events API now rejects duplicate scalar query parameters and enforces canonical integer formatting for the limit parameter, returning clear validation errors.
• Tests
  • Added a test verifying ambiguous or non-canonical query parameters produce 400 responses and that a canonical request succeeds.

[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: 88cae7be-3602-44b6-8526-dc32ed250471

📥 Commits

Reviewing files that changed from the base of the PR and between 28eaba1 and 7a6b512.

📒 Files selected for processing (2)

• app/bounty_api.py
• tests/test_bounty_api.py

---

📝 Walkthrough

Walkthrough

The PR adds query parameter validation to the /api/v1/admin/webhook-events endpoint: it rejects repeated status or limit query parameters and enforces canonical integer formatting for limit. A test exercises repeated status, repeated limit, non-canonical limit=01, and a valid canonical request.

Changes

Query Parameter Validation

Layer / File(s)	Summary
Webhook-events query parameter validation and tests app/bounty_api.py, tests/test_bounty_api.py	api_admin_webhook_events validates incoming status and limit parameters, rejecting repeated values and non-canonical integers before database lookup. Test covers repeated status, repeated limit, non-canonical limit=01, and confirms valid canonical parameters return 200.

Possibly related issues

• Proposed work: validate admin webhook limit query forms #817: Adds the same repeated and non-canonical limit raw-query guards for the webhook-events handler.
• Proposed work: validate remaining admin scalar query forms #818: Related work proposing repeated status rejection for /api/v1/admin/webhook-events.

Possibly related PRs

• ramimbo/mergework#824: Modifies the same webhook-events handler to tighten admin scalar query validation (related changes to status/limit validation).
• Reject leading-zero numeric path aliases #790: Prior work adding canonical positive-integer validation used elsewhere (overlaps on limit canonicalization).
• Reject non-canonical numeric query aliases #769: Earlier PR introducing reject_noncanonical_int_query_param and related integer-query validation.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and concisely describes the main change: rejecting ambiguous query parameters in the admin webhook events API.
Description check	✅ Passed	The description covers the problem, specific changes, validation steps, and bounty reference, though it omits the optional Evidence and Test Evidence checklist sections from the template.
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 changes only add query validation logic and tests with no new investment, price, cash-out claims, or fabricated payouts in README, docs, or code comments.
Bounty Pr Focus	✅ Passed	PR diff matches stated bounty scope: 2 files, 33 lines targeting /api/v1/admin/webhook-events validation for issue #656. Tests cover all requirements with negative/positive cases. 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.}

[jakerated-r]

Thanks for the focused admin-webhook-events hardening. I reviewed current head 28eaba193f101e720556f89a3c9707e2fd996474 against current origin/main 53d73feef2ad8a4e44571863c73375d7193c9019.

I do not think this should merge as-is because the PR is currently dirty against main:

git merge-tree --write-tree origin/main origin/pr/835

Auto-merging app/bounty_api.py
CONFLICT (content): Merge conflict in app/bounty_api.py

The conflict is in app/bounty_api.py around /api/v1/admin/webhook-events. Current main already has request: Request and reject_repeated_query_param(request, "status") on that route, while this PR was branched from an older base and adds the adjacent limit checks. The useful remaining delta appears to be the limit duplicate/noncanonical validation and its regression test, but the head needs to be rebased or otherwise reconciled before maintainers can safely evaluate the final merged route.

One more bounty-lifecycle note: the PR body references #656, but #656 is now closed/paid with 0 effective awards remaining. That does not block the code review itself, but it should not be treated as a live #656 payout claim after rebase.

Validation evidence I captured:

• gh pr view 835 reports mergeStateStatus=DIRTY, hosted Quality success, and CodeRabbit success/no actionable comments on the current head.
• git diff --stat origin/main...origin/pr/835 shows a narrow 2-file delta: app/bounty_api.py and tests/test_bounty_api.py.
• git merge-tree --write-tree origin/main origin/pr/835 reproduces the merge conflict in app/bounty_api.py.
• I did not use local pytest/Ruff as a pass/fail signal because the detached review worktree on this machine does not have the project dependencies installed (fastapi missing, Ruff unavailable there).

[gshaowei6]

Reviewed current head 28eaba193f101e720556f89a3c9707e2fd996474 as a non-author against current origin/main 53d73feef2ad8a4e44571863c73375d7193c9019.

The patch itself is focused: app/bounty_api.py adds the limit duplicate/non-canonical guards on /api/v1/admin/webhook-events, and tests/test_bounty_api.py covers repeated status, repeated limit, limit=01, and the canonical happy path.

Branch-local validation passed:

• pytest tests/test_bounty_api.py -q -> 19 passed, 1 warning
• ruff check app/bounty_api.py tests/test_bounty_api.py -> passed
• ruff format --check app/bounty_api.py tests/test_bounty_api.py -> 2 files already formatted
• python scripts/docs_smoke.py -> docs smoke ok
• git diff --check origin/main...HEAD -> clean

The current integration blocker is mergeability against latest main: git merge-tree --write-tree origin/main HEAD fails with a content conflict in app/bounty_api.py. Current main already updated api_admin_webhook_events() to accept Request and reject repeated status, while this PR adds the limit duplicate/non-canonical checks in the same block. Please rebase/merge current main and preserve both the existing status guard and the new limit guards/tests, then rerun the focused bounty API tests and formatting checks.

[keilogic]

Thanks for rebasing this. I reviewed current head 7a6b51213c5f75e215749b92e65f552a0ded4908 against current origin/main 53d73feef2ad8a4e44571863c73375d7193c9019.

The earlier mergeability blocker is resolved: current main already has the Request argument and repeated-status guard, and this head preserves that while adding the repeated/non-canonical limit guards. The final diff is narrow: app/bounty_api.py adds the limit validation calls, and tests/test_bounty_api.py covers repeated status, repeated limit, limit=01, and the canonical happy path.

Validation I ran:

• pytest tests/test_bounty_api.py -q -> 19 passed, 1 warning
• ruff check app/bounty_api.py tests/test_bounty_api.py -> passed
• ruff format --check app/bounty_api.py tests/test_bounty_api.py -> 2 files already formatted
• python scripts/docs_smoke.py -> docs smoke ok
• mypy app/bounty_api.py -> success
• git diff --check origin/main...HEAD -> clean
• git merge-tree --write-tree origin/main HEAD -> clean tree d271eb21fed62e9a6594d25479218cd572ca17be

Hosted Quality and CodeRabbit are successful on this head, and GitHub reports mergeStateStatus=CLEAN.

No remaining code blocker found. One bounty-lifecycle note remains: #656 is closed/paid with no effective awards left, so this should not be treated as a live #656 payout claim.

[jakerated-r]

Follow-up on my earlier changes-requested review at head 28eaba193f101e720556f89a3c9707e2fd996474: I rechecked the current head 7a6b51213c5f75e215749b92e65f552a0ded4908 after the rebase.

The prior mergeability blocker is resolved now. Evidence from the fresh check:

• origin/main: 53d73feef2ad8a4e44571863c73375d7193c9019
• origin/pr/835: 7a6b51213c5f75e215749b92e65f552a0ded4908
• git merge-tree --write-tree origin/main origin/pr/835 exits clean and produced tree d271eb21fed62e9a6594d25479218cd572ca17be
• git diff --stat origin/main...origin/pr/835 is now the focused 2-file / 33-line delta in app/bounty_api.py and tests/test_bounty_api.py
• git diff --check origin/main...origin/pr/835 is clean
• Hosted Quality check is successful on the PR page

I am not posting this as a separate bounty claim; this is just a follow-up receipt on the blocker I raised earlier so maintainer accounting can see the prior issue is no longer present.

[aunysillyme]

Verified the changes on branch xtakenotesx16:fix/admin-webhook-events-query-validation (head: 7a6b512) locally.

• Checked route /api/v1/admin/webhook-events parameter validation. It successfully applies reject_repeated_query_param and reject_noncanonical_int_query_param to limit as intended.
• Ran the unit test suite (pytest tests/test_bounty_api.py), and all 19 tests pass successfully.
• Code is formatted cleanly and passes ruff check with zero errors.

[ramimbo]

Maintainer update: holding this as superseded by merged #823. The useful /api/v1/admin/webhook-events limit validation is now on main through #823, which also covered the related /admin?webhook_limit=... path.

Bounty note: #656 is closed/exhausted, so there is no live #656 payable path. I did not route this to #799 because this PR did not target that live bounty or link an exact #799 source under its submission rules. No mrwk:accepted label was applied and no payment proposal was created.