Validate OAuth scalar query parameters

[RLTree]

Summary

• reject repeated next on the GitHub OAuth login route before the signed state is built
• reject repeated code and state on the GitHub OAuth callback route before cookie-state comparison or outbound token exchange
• add regression tests for duplicate callback parameters, no token exchange on duplicate callback input, and the normal single-value callback path

Source issue #820: #820
Related proposed-work bounty #722: #722

Maintainer hold repair: this PR no longer claims #761. Issue #820 is a proposed-work item related to #722, and no payout is queued for #820 while that round is full.

Refs #820

Validation

• uv run --extra dev pytest tests/test_wallet_api.py::test_github_callback_rejects_repeated_scalar_queries tests/test_wallet_api.py::test_github_callback_exchanges_single_scalar_queries tests/test_wallet_api.py::test_github_login_rejects_repeated_next_query -q -> 5 passed, 1 warning
• uv run --extra dev pytest tests/test_wallet_api.py tests/test_security.py::test_oauth_next_path_rejects_external_or_headerlike_paths -q -> 61 passed, 1 warning
• uv run --extra dev ruff format --check app/auth.py tests/test_wallet_api.py -> 2 files already formatted
• uv run --extra dev ruff check app/auth.py tests/test_wallet_api.py -> All checks passed
• uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
• uv run --extra dev mypy app -> Success: no issues found in 39 source files
• git diff --check -> clean

Live duplicate/capacity check before PR:

• gh issue view 820 -R ramimbo/mergework --json number,title,state,url,body,comments,labels,updatedAt -> Proposed work: validate OAuth scalar query parameters #820 open, labeled proposed-work
• gh issue view 722 -R ramimbo/mergework --json number,title,state,url,comments,labels,updatedAt -> MRWK bounty: 50 MRWK - accepted proposed-work requests, round 2 #722 open; maintainer comment says Proposed work: validate OAuth scalar query parameters #820 is left for successor intake review and no pay_bounty proposal is queued because MRWK bounty: 50 MRWK - accepted proposed-work requests, round 2 #722 has 0 effective slots remaining
• gh pr view 825 -R ramimbo/mergework --json number,title,state,url,mergeStateStatus,statusCheckRollup,comments,body,headRefName,headRepositoryOwner,updatedAt -> PR Validate OAuth scalar query parameters #825 open and clean; maintainer hold requested corrected bounty/source reference plus no-cookie/no-Location assertions for repeated next
• gh pr list -R ramimbo/mergework --state open --search "820 OR OAuth scalar OR repeated next OR repeated state OR repeated code OR github callback OR github login" --json number,title,author,url,headRefName,updatedAt,body -> no OAuth scalar duplicate PR found

No wallet, payout, treasury execution, bridge, exchange, cash-out, price, secrets, private details, or issue automation changes.

Summary by CodeRabbit

• Bug Fixes

  • GitHub login and callback endpoints now reject requests with duplicated or malformed OAuth query parameters (e.g., repeated next, code, or state), preventing invalid or ambiguous authentication attempts.

• Tests

  • Added end-to-end tests covering OAuth query validation and successful single-parameter authentication flows, including cookie and redirect behavior verification.

[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: e9d4a1ef-a840-4b96-bdbf-887e2937dea2

📥 Commits

Reviewing files that changed from the base of the PR and between 33e583a and 9e12d3c.

📒 Files selected for processing (1)

• tests/test_wallet_api.py

---

📝 Walkthrough

Walkthrough

This PR adds validation to reject repeated query parameters on GitHub OAuth routes. The /auth/github/login endpoint now rejects requests where the next parameter appears more than once, and /auth/github/callback rejects requests with repeated code or state parameters. Comprehensive tests verify these validations return HTTP 400 and that valid single-instance exchanges proceed correctly.

Changes

OAuth repeated query parameter validation

Layer / File(s)	Summary
OAuth route query parameter validation app/auth.py	Import reject_repeated_query_param and add validation to /auth/github/login (reject repeated next before state generation) and /auth/github/callback (reject repeated code/state before token exchange).
Tests for repeated query parameter rejection tests/test_wallet_api.py	Test /auth/github/login returns 400 for repeated next; parametrized test confirms /auth/github/callback returns 400 for repeated scalar params without token exchange and preserves mrwk_oauth_state cookie; separate test validates single-instance code/state exchange succeeds with correct redirects and cookies.

Possibly related issues

• ramimbo/mergework#820: Implements the repeated query parameter rejection proposal for OAuth scalar parameters (next, code, state) on login and callback routes.

Possibly related PRs

• ramimbo/mergework#400: Established the GitHub OAuth route handlers in app/auth.py that are now extended with query validation.
• ramimbo/mergework#771: Introduced the shared reject_repeated_query_param utility that this PR applies to OAuth-specific query parameters.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Validate OAuth scalar query parameters' directly names the changed surface and clearly summarizes the main security fix applied to the auth routes.
Description check	✅ Passed	The description covers all required sections: summary of changes, evidence of the issue, test evidence with validation commands and results, and related bounty/issue references.
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	Code changes contain no price, investment, cash-out, or fabricated payout claims; README correctly describes MRWK as native coin with future snapshot/bridge support.
Bounty Pr Focus	✅ Passed	PR diff matches stated scope: OAuth scalar query parameter validation added to login/callback routes with three regression tests. No unrelated changes detected.

_{✏️ 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: 74d05e19-ec7b-4a85-a0e0-8e5cd7d5a84a

📥 Commits

Reviewing files that changed from the base of the PR and between 12ca2f0 and 33e583a.

📒 Files selected for processing (2)

• app/auth.py
• tests/test_wallet_api.py

[keilogic]

Reviewed head 33e583a74d32ef33848d4c662efb97da79b37a32 against origin/main 12ca2f0510abce4f3a8b3d77ae8596f47fa5c74c.

The code path itself is narrowly scoped and the validation I ran passed:

• python -m pytest tests/test_wallet_api.py tests/test_security.py::test_oauth_next_path_rejects_external_or_headerlike_paths -q -> 61 passed, 1 warning
• python -m ruff check app/auth.py tests/test_wallet_api.py -> passed
• python -m ruff format --check app/auth.py tests/test_wallet_api.py -> passed
• python scripts/docs_smoke.py -> docs smoke ok
• git diff --check origin/main...HEAD -> clean
• git merge-tree --write-tree origin/main HEAD -> clean tree d9d5d5d6d7de5aaa7889580d4e95d273f1621d28

I see one concrete test issue to fix before this should be treated as ready:

test_github_login_rejects_repeated_next_query should also assert that the duplicate-next rejection does not emit mrwk_oauth_state and does not set a Location header. The current implementation rejects before state generation, but the test would not catch a regression where state or redirect side effects are produced before returning 400.

On bounty/source eligibility, the PR references Bounty #761 and source issue #820; maintainers have since put this PR on hold for #761 pending source/bounty-reference correction, so I am not treating that as a code finding beyond the maintainer eligibility decision.

No other blocker found in the touched OAuth validation code.

[ramimbo]

Maintainer hold: this is not accepted for #761 as-is. The current-head review requests two fixes before maintainer acceptance: correct the bounty/source reference for the OAuth work, and assert the repeated-next rejection does not emit an OAuth state cookie or Location header. No payout is queued for this PR.

[RLTree]

Maintainer hold repair pushed in 9e12d3c.

Changes:

• corrected the PR body/source reference: Proposed work: validate OAuth scalar query parameters #820 is a proposed-work item related to MRWK bounty: 50 MRWK - accepted proposed-work requests, round 2 #722, not MRWK bounty: 150 MRWK - focused fixes from confirmed project needs, round 3 #761
• added assertions that repeated next rejection emits no Location header and no mrwk_oauth_state cookie

Validation:

• uv run --extra dev pytest tests/test_wallet_api.py::test_github_login_rejects_repeated_next_query tests/test_wallet_api.py::test_github_callback_rejects_repeated_scalar_queries tests/test_wallet_api.py::test_github_callback_exchanges_single_scalar_queries -q -> 5 passed, 1 warning
• uv run --extra dev pytest tests/test_wallet_api.py tests/test_security.py::test_oauth_next_path_rejects_external_or_headerlike_paths -q -> 61 passed, 1 warning
• uv run --extra dev ruff format --check app/auth.py tests/test_wallet_api.py -> 2 files already formatted
• uv run --extra dev ruff check app/auth.py tests/test_wallet_api.py -> All checks passed
• uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
• uv run --extra dev mypy app -> Success: no issues found in 39 source files
• git diff --check -> clean

No payout is claimed or queued for #820 while #722 is full.

[jakerated-r]

Reviewed current head 9e12d3c1bc7cac182533d956631b1f03aaee52aa against current origin/main 8f197562f3e6fe5d1a61a57456c36eda4309b0d1 after the maintainer-hold repair.

I inspected app/auth.py and tests/test_wallet_api.py, including the delta from the prior reviewed head 33e583a74d32ef33848d4c662efb97da79b37a32 to 9e12d3c. That delta is limited to the expected test hardening: duplicate-next rejection now asserts no Location header and no mrwk_oauth_state cookie.

Validation passed:

• PYTHONPATH=. uv run --extra dev pytest tests/test_wallet_api.py::test_github_login_rejects_repeated_next_query tests/test_wallet_api.py::test_github_callback_rejects_repeated_scalar_queries tests/test_wallet_api.py::test_github_callback_exchanges_single_scalar_queries -q -> 5 passed, 1 warning
• PYTHONPATH=. uv run --extra dev pytest tests/test_wallet_api.py tests/test_security.py::test_oauth_next_path_rejects_external_or_headerlike_paths -q -> 61 passed, 1 warning
• PYTHONPATH=. uv run --extra dev ruff check app/auth.py tests/test_wallet_api.py -> passed
• PYTHONPATH=. uv run --extra dev ruff format --check app/auth.py tests/test_wallet_api.py -> passed
• PYTHONPATH=. uv run --extra dev mypy app/auth.py -> passed
• PYTHONPATH=. uv run python scripts/docs_smoke.py -> docs smoke ok
• git diff --check origin/main...HEAD -> clean
• git merge-tree --write-tree origin/main HEAD -> clean tree 9778140587eddbdea7c23755971d4dfe753d2ab5

Direct duplicate-next probe also returned HTTP 400 with detail next must be provided at most once, no Location header, and no mrwk_oauth_state cookie.

Hosted Quality, readiness, docs, and image checks and CodeRabbit are successful on this head; GitHub reports mergeStateStatus=CLEAN. The PR body now correctly says #820 is related to #722 and that no payout is claimed or queued for #820 while #722 is full.

No remaining blocker found in the touched OAuth scalar query validation path. No private data, wallet material, payout execution, treasury mutation, bridge, exchange, cash-out, MRWK price behavior, or #820 payout claim was used.

[xiefuzheng713-alt]

Reviewed current head 9e12d3c1bc7cac182533d956631b1f03aaee52aa as a non-author current-head review.

I did not find a remaining blocker in the OAuth scalar query validation path. The change stays narrow: /auth/github/login rejects repeated next before building OAuth state, and /auth/github/callback rejects repeated code / state before state comparison or token exchange.

Evidence checked:

• inspected app/auth.py and confirmed the repeated-query checks are only added to the GitHub OAuth login/callback paths;
• inspected tests/test_wallet_api.py and confirmed duplicate next asserts HTTP 400, no Location, and no mrwk_oauth_state cookie;
• confirmed duplicate callback code / state tests keep the OAuth client uncalled and preserve the existing cookie state;
• confirmed the single-scalar callback test still exercises the normal token/user exchange and clears mrwk_oauth_state.

Validation I ran:

• uv run --extra dev pytest tests/test_wallet_api.py tests/test_security.py::test_oauth_next_path_rejects_external_or_headerlike_paths -q -> 61 passed, 1 warning
• uv run --extra dev ruff check app/auth.py tests/test_wallet_api.py -> passed
• uv run --extra dev ruff format --check app/auth.py tests/test_wallet_api.py -> 2 files already formatted
• uv run --extra dev mypy app/auth.py -> success
• uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
• git diff --check origin/main...HEAD -> clean
• git merge-tree --write-tree origin/main HEAD -> clean tree dc6cf63f7239e886a45ec9ba479f2aee54f6961e

GitHub state checked: mergeStateStatus=CLEAN; hosted Quality/readiness/docs/image and CodeRabbit checks are successful on this head. No private data, wallet material, payout execution, treasury mutation, exchange, bridge, cash-out, MRWK price behavior, or issue mutation was used.