Reject boolean bounty identifiers#245
Closed
TUPM96 wants to merge 1 commit into
Closed
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR tightens validation around integer identifiers used in ledger bounty operations, ensuring booleans (which are int subclasses in Python) are rejected and adding regression tests for those cases.
Changes:
- Added
_positive_inthelper to validate “positive, non-boolean int” inputs and reused it increate_bounty,pay_bounty, andclose_bounty. - Updated
find_bounty_by_issueto validateissue_numbervia_positive_int. - Added tests to ensure boolean
issue_number,max_awards, andbounty_idare rejected with clearLedgerErrors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_ledger.py | Adds regression tests covering boolean inputs for issue numbers, max awards, and bounty IDs. |
| app/ledger/service.py | Centralizes positive-int validation and applies it to bounty creation and mutation paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+152
to
+157
| def _positive_int(value: int, field: str) -> int: | ||
| if isinstance(value, bool) or not isinstance(value, int): | ||
| raise LedgerError(f"{field} must be an integer") | ||
| if value <= 0: | ||
| raise LedgerError(f"{field} must be positive") | ||
| return value |
Comment on lines
431
to
437
| def find_bounty_by_issue(session: Session, repo: str, issue_number: int) -> Bounty | None: | ||
| clean_issue_number = _positive_int(issue_number, "issue_number") | ||
| return session.scalar( | ||
| select(Bounty).where(Bounty.repo == repo, Bounty.issue_number == issue_number).limit(1) | ||
| select(Bounty) | ||
| .where(Bounty.repo == repo, Bounty.issue_number == clean_issue_number) | ||
| .limit(1) | ||
| ) |
MolhamHamwi
reviewed
May 25, 2026
Contributor
MolhamHamwi
left a comment
MolhamHamwi
left a comment
There was a problem hiding this comment.
No blockers found.
Evidence checked:
- Inspected
app/ledger/service.pyand confirmed_positive_int()rejects booleans/non-integers beforeTruecan alias issue or bounty id1, while still preserving the existing positive checks and themax_awardsupper bound. - Confirmed
create_bounty()now uses cleanedissue_number/max_awardsvalues consistently for duplicate lookup, reservation math, and persisted bounty fields. - Confirmed
find_bounty_by_issue(),pay_bounty(), andclose_bounty()all pass their ids through the same guard before database lookup or ledger mutation. - Inspected
tests/test_ledger.pycoverage for booleanissue_number, booleanmax_awards, booleanbounty_idon payout/close, and the post-failure state checks that the bounty remains open and Alice receives no balance. - Ran local validation on PR head
92c848c: focused boolean-id tests ->2 passed; broader ledger/webhook suite ->31 passed; Ruff check passed; Ruff format check already formatted;git diff --check origin/main...HEAD-> clean. - Checked GitHub shows clean merge state and hosted
Quality, readiness, docs, and image checkssuccessful.
No secrets, wallet material, private deployment values, payout details, or MRWK price claims were used.
ayskobtw-lil
approved these changes
May 25, 2026
Contributor
ayskobtw-lil
left a comment
ayskobtw-lil
left a comment
There was a problem hiding this comment.
No blockers from my review.
Evidence checked:
- Inspected
app/ledger/service.pyandtests/test_ledger.py;_positive_int()rejects booleans before Python can treatTrueas id/issue1, while preserving the existing positive checks andmax_awardsupper bound. - Confirmed
create_bounty()now stores and searches by the cleanedissue_number/max_awardsvalues, so duplicate detection and reserve sizing use the validated integers. - Confirmed
find_bounty_by_issue(),pay_bounty(), andclose_bounty()validate direct service-layer identifiers before reading or mutating bounty rows. - Verified the mutation regression leaves the bounty open with
awards_paid == 0and no payout balance after booleanbounty_idattempts. - Ran
python -m pytest tests/test_ledger.py::test_create_bounty_rejects_boolean_issue_number_or_awards tests/test_ledger.py::test_bounty_mutations_reject_boolean_bounty_id tests/test_ledger.py::test_create_bounty_rejects_non_positive_issue_number tests/test_ledger.py::test_close_bounty_releases_unpaid_awards -q-> 4 passed. - Ran
python -m ruff check app/ledger/service.py tests/test_ledger.py-> passed. - Ran
python -m ruff format --check app/ledger/service.py tests/test_ledger.py-> 2 files already formatted. - Ran
git diff --check origin/main...HEAD-> clean.
No secrets, wallet material, deployment values, payout details, private vulnerability details, or MRWK price claims were reviewed or disclosed.
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #228
Summary
1.issue_number,max_awards, andbounty_idvalues.Repro Before Fix
A direct service call with
issue_number=Trueorbounty_id=Truewas accepted because Python treatsTrueas integer1. That could create a bounty for issue#1, or pay/close bounty id1, instead of rejecting malformed identifiers.Verification
uv run --python 3.12 --extra dev python -m pytest tests/test_ledger.py -q-> 18 passeduv run --python 3.12 --extra dev python -m pytest -q-> 207 passed, 2 warningsuv run --python 3.12 --extra dev ruff check .-> all checks passeduv run --python 3.12 --extra dev ruff format --check .-> 37 files already formatteduv run --python 3.12 --extra dev python -m mypy app-> successuv run --python 3.12 --extra dev python scripts/docs_smoke.py-> docs smoke okuv run --python 3.12 --extra dev python scripts/check_agents.py-> AGENTS.md okgit diff --check-> cleanNo secrets, wallet material, private vulnerability details, deployment values, payout details, PayPal details, or MRWK price claims are included.