Skip to content

fix: input validation and missing auth (Batch #69)#4159

Open
BossChaos wants to merge 2 commits intoScottcjn:mainfrom
BossChaos:sec-batch69
Open

fix: input validation and missing auth (Batch #69)#4159
BossChaos wants to merge 2 commits intoScottcjn:mainfrom
BossChaos:sec-batch69

Conversation

@BossChaos
Copy link
Copy Markdown
Contributor

fix: input validation and missing auth (Batch #69)

  • Add input length limits to contributor_registry.py (prevent DoS via oversized inputs)
  • Add admin authentication to payout_ledger.py PATCH /status endpoint
  • Prevent unauthorized status modifications to payout records

Co-Authored-By: Hermes Agent hermes@nous.research

BossChaos and others added 2 commits May 5, 2026 02:52
- Add input length limits to contributor_registry.py (prevent DoS via oversized inputs)
- Add admin authentication to payout_ledger.py PATCH /status endpoint
- Prevent unauthorized status modifications to payout records

Co-Authored-By: Hermes Agent <hermes@nous.research>
@BossChaos BossChaos requested a review from Scottcjn as a code owner May 8, 2026 10:59
@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) ci size/XS PR: 1-10 lines labels May 8, 2026
Copy link
Copy Markdown

@jujujuda jujujuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: PR #4159 — input validation and missing auth

Reviewer: jujujuda (Atlas bounty hunter)
Bounty Program: #73 Code Review Bounty

Summary

Two independent fixes bundled in one PR. Both are correct.

Fix 1: contribution_history truncation (contributor_registry.py)

  • [:500] is appropriate for a text field — prevents DB bloat from oversized input
  • Minor: (request.form.get(...) or ) is redundant when `` is already the default, but harmless
  • Verdict: Correct, non-controversial

Fix 2: Ledger admin auth (payout_ledger.py)

  • Bearer token check is the right primitive for a REST API
  • Using os.environ.get is appropriate — avoids hardcoding secrets
  • Missing: should also validate the record belongs to the caller (IDOR concern), but that may be out of scope for this PR
  • Verdict: Solid auth fix

Minor Pattern Issue

Both workflow YAML changes are just comments being disabled, not related to the code fixes. Consider splitting into a separate chore PR next time for cleaner blame.

Verdict

Standard Review: 7/10 RTC — Both fixes are correct and address real security/robustness issues. No concerns.


Claiming under Bounty #73 | Wallet: RTC2fe3c33c77666ff76a1cd0999fd4466ee81250ff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) ci size/XS PR: 1-10 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants