-
-
Notifications
You must be signed in to change notification settings - Fork 18
Add composite BLT bounty processor and /bounty issue_comment workflow #100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded@Jayant2908 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new GitHub Actions composite action Changes
Sequence Diagram(s)sequenceDiagram
participant WF as GitHub Workflow
participant CA as BLT Bounty Processor
participant BLT as BLT API
participant GH as GitHub API
WF->>CA: invoke action (inputs: base_url, token, issue_url, amount, commenter, repo, github_token)
CA->>CA: validate & normalize amount
CA->>BLT: POST /bounties (payload: issue_url, amount, sponsor, metadata)
BLT-->>CA: 2xx create response (body)
CA->>BLT: GET /bounties/total?issue=<url-encoded>
BLT-->>CA: total or error
CA->>CA: resolve total (use fallback if GET fails)
CA->>GH: remove existing "$X Bounty" labels from issue
GH-->>CA: removal result
CA->>GH: ensure/create "$<total> Bounty" label and apply to issue
GH-->>CA: label apply result
CA->>GH: post confirmation comment with sponsor and total
GH-->>CA: comment result
CA-->>WF: outputs (normalized_amount, raw_create_response, resolved_total)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
Bounty_Processor/action.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a composite GitHub Action for managing BLT bounties directly from GitHub issues. It enables maintainers and sponsors to use /bounty commands in issue comments to create bounties, which are then tracked via the BLT API and reflected in issue labels and confirmation comments.
Key Changes:
- Added
Bounty_Processorcomposite action with bounty amount validation, BLT API integration for creating/querying bounties, and automated label/comment management - Implements a four-step workflow: validate amount format, create bounty via API, query total bounty amount, and update GitHub issue with labels and confirmation comment
- Uses inline Python scripts for JSON manipulation and URL encoding within bash steps
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Bounty_Processor/action.yml (1)
47-50: Add validation to reject zero or negative bounty amounts.The regex at line 47 allows
$0or$0.00, which doesn't make business sense for a bounty. Add a numeric check to ensure the amount is greater than zero before accepting the input.Apply this diff to add the check:
if ! echo "$AMOUNT" | grep -Eq '^[0-9]+(\.[0-9]{1,2})?$'; then echo "Invalid amount format: $AMOUNT" >&2 exit 1 fi + # Ensure amount is strictly positive + if (( $(echo "$AMOUNT <= 0" | bc -l) )); then + echo "Amount must be greater than zero: $AMOUNT" >&2 + exit 1 + fi echo "amount=$AMOUNT" >> "$GITHUB_OUTPUT"
🧹 Nitpick comments (2)
Bounty_Processor/action.yml (2)
86-86: Consider normalizing base URL to avoid double slashes.Lines 86 and 128 construct API endpoints by appending to
$API_BASE_URLwithout removing potential trailing slashes. While most servers handle//bounties/gracefully, it's cleaner to normalize:- "$API_BASE_URL/bounties/") + "${API_BASE_URL%/}/bounties/")and similarly for line 128.
Also applies to: 128-128
1-255: Test coverage for composite action.This composite action orchestrates four critical steps (validation, API calls, label management, comments) but lacks automated test coverage. The repository has comprehensive Mocha and integration tests for other features; this new functionality should have similar coverage for amount validation, BLT API interactions, label idempotency, and error paths.
Would you like help generating a test suite for this action (end-to-end validation, happy path, error scenarios)? This can be added in a follow-up PR as mentioned, or integrated here if preferred.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
Bounty_Processor/action.yml(1 hunks)README.md(3 hunks)
🔇 Additional comments (3)
README.md (1)
43-46: Well-documented bounty feature addition.The README additions clearly explain the
/bountycommand, provide a complete workflow example with helpful parsing logic, and document the integration with both GitHub and Slack. The input descriptions (especially lines 191-192 clarifying token format) help users avoid mistakes.Also applies to: 156-200, 247-265
Bounty_Processor/action.yml (2)
37-106: Well-structured API integration with robust error handling.The three-step flow (validate, create via API, query total) is clear and handles errors appropriately:
- Validated amount is consistently used downstream
- HTTP status codes are checked and failures propagate
- Sensitive data (payloads, responses) is handled carefully with temp files and redacted logging
- Fallbacks are in place for API query failures
160-255: Label management and confirmation flow is well-implemented.The GitHub Script step cleanly separates concerns:
- Env vars are safely passed and read into JavaScript variables
- The total is validated before use in label names
- Label removal gracefully handles the pattern (old bounty labels)
- Label creation is idempotent (ignores "already exists" errors)
- Critical operations fail appropriately (core.setFailed for comment posting)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Bounty_Processor/action.yml (2)
131-131: Inconsistent API_BASE_URL normalization between steps.Line 91 (create_bounty) normalizes the base URL with
${API_BASE_URL%/}to strip trailing slashes, but line 131 (get_total) uses the raw$API_BASE_URLwithout this check. If the input ends with a trailing slash, this will produce double slashes in the query URL (e.g.,https://example.com/api//bounties/issue-total/), which could fail on strict API gateways.Apply this diff to normalize consistently:
http_code=$(curl -sS -w "%{http_code}" -o "$http_response" \ -H "Authorization: Token $BLT_API_TOKEN" \ - "$API_BASE_URL/bounties/issue-total/?github_issue_url=$ISSUE_URL_ENC") + "${API_BASE_URL%/}/bounties/issue-total/?github_issue_url=$ISSUE_URL_ENC")
185-258: Error handling design: partial failures in label operations won't block comment.The step has intentionally tiered error handling: label operations (remove, create, add) log warnings/info and continue, while comment posting is the only hard-stop (via
core.setFailed). This means if all label operations fail but the comment posts successfully, users will see "bounty added" confirmation without the label being present.This is a reasonable MVP design to ensure users are always notified via comment, but consider adding a follow-up check: verify that the label was actually added to the issue before posting the confirmation comment, or escalate label operation failures to
core.setFailedif label presence is critical to the feature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
Bounty_Processor/action.yml(1 hunks)
🔇 Additional comments (1)
Bounty_Processor/action.yml (1)
1-35: Well-structured composite action with clear inputs and outputs.The action definition is clean, all required inputs are documented with helpful descriptions, and the step structure is logical: validate → create → query total → update labels. The previous issues around token handling and amount validation have been properly addressed.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8329 @Pritz395: I've verified all 21 findings against the actual Verification Results✅ CRITICAL ISSUES - All 4 Verified
✅ BUGS - 3 of 4 Verified
✅ CODE QUALITY - All 5 Verified
✅ LOGIC ISSUES - All 3 Verified
✅ SECURITY - Both Verified
✅ PRODUCTION READINESS - All 3 Verified
SummaryVerification Score: 20/21 Valid
Priority Fixes (in order):
Excellent analysis! The findings are thorough, well-documented, and actionable. |
Pritz395
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Reviewed your code and found some issues:
Critical:
- URL encoding bug (line 115) - using safe="" encodes slashes and colons, which breaks GitHub URLs. Need to use safe="/:"
- Missing bc dependency (line 47) - bc -l might not be available on all systems. Should use awk or bash arithmetic instead
- Race conditions - concurrent /bounty comments can create inconsistent labels. Need concurrency control in the calling workflow
- Python dependency - Python is used but there's no check if it's actually available. Either add verification or switch to jq
Bugs:
- No input validation for issue_url, commenter, and issue_number
- Temp file cleanup - missing trap to ensure cleanup happens on unexpected exit
- Comment posting failure is fatal - should use core.warning() instead of core.setFailed() to avoid leaving things in an inconsistent state
Code quality:
- Missing curl timeouts - add --max-time 30 to prevent hangs
- Python heredoc syntax issue (line 152) - <<<"$resp" isn't portable across shells. Use echo "$resp" | python - instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
Bounty_Processor/action.yml(1 hunks)
🔇 Additional comments (6)
Bounty_Processor/action.yml (6)
37-56: ✓ Amount validation properly implemented.The step correctly validates format (regex), ensures positive value (awk check), and outputs normalized amount. Use of awk instead of bc addresses the portability concern from prior review.
60-131: ✓ Bounty creation step well-implemented.Properly uses validated amount, includes Python availability check, validates inputs (issue_url format, issue_number numeric, commenter non-empty), uses curl with timeouts and retries, checks HTTP status codes, and avoids logging sensitive payloads. Resource cleanup with trap is good practice.
134-167: ✓ Query total step setup properly configured.Correctly uses validated amount as fallback, includes Python availability check, URL encoding with safe="/:/" preserves GitHub URL structure, curl configured with timeouts/retries matching the create bounty step, and HTTP status code handling is defensive.
191-192: ✓ Output format correct.Once the syntax errors in the preceding Python heredoc are fixed, this output section will properly expose the total for downstream steps.
207-222: ✓ Input handling and validation properly secured.Uses
process.envto safely read inputs (avoiding direct interpolation injection risks), validates total from BLT API against numeric pattern, and falls back to validated input amount if invalid. Proper use of core.warning for logging anomalies.
223-282: ✓ Label management properly implemented with retries.Retry function provides exponential backoff (0.5s, 1s, 1.5s, etc.) for transient failures. Label pattern is precise and case-sensitive. All label operations (remove, create, add) wrapped in retry logic with graceful error handling using core.warning. Hardcoded color "ffd700" is acceptable for now.
Note: Label removal and addition remain non-atomic (possible race with concurrent bounty commands on same issue), but as discussed in prior review, this is mitigated by workflow-level concurrency controls in the calling workflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Bounty_Processor/action.yml (1)
170-187: Fix:get_totalalways falls back because Python reads empty stdin.
total=$(python - <<'PY' ...)usesjson.load(sys.stdin)but nothing is piped into stdin, so it will throw and print the fallback every time.- total=$(python - <<'PY' + total=$(printf '%s' "$resp" | python - <<'PY' import json, os, sys fallback = os.environ.get("FALLBACK_AMOUNT", "0") try: data = json.load(sys.stdin) value = data.get("total_amount", data.get("total")) if value in (None, ""): print(fallback) else: print(str(value)) except Exception: print(fallback) PY )
🧹 Nitpick comments (2)
Bounty_Processor/action.yml (2)
105-115: Simplify tmpfile cleanup (avoid trap + manual rm duplication).You already
rm -f "$tmpfile"after curl; keeping thetraptoo is redundant. Either rely on the trap (and remove the manual rm) or remove the trap and keep explicit cleanup.tmpfile=$(mktemp) - trap 'rm -f "$tmpfile"' EXIT printf '%s' "$payload" > "$tmpfile" ... response_and_status=$(curl ... ) rm -f "$tmpfile"
211-265: Normalizetotalfor stable labels (avoid$10,$10.0,$10.00churn) + guard length.Even with the regex, different string representations can create multiple labels for the same numeric value; also consider capping label name length to stay within GitHub limits.
Suggested approach: parse to number, format to 2dp, then optionally trim
.00(or pick one canonical format and stick to it).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
Bounty_Processor/action.yml(1 hunks)
|
This wouldn’t work in that folder feel free to try again it has to go into .github folder |
|
@coderabbitai Suggest how should I move action.yml in .github folder so that it works perfectly!? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1181 🏁 Script executed: Length of output: 2401 🏁 Script executed: Length of output: 4485 @Jayant2908: I've analyzed your repository structure, and I need to clarify the situation: Current StatusThe When you reference it as What's MissingHowever, I notice you're missing the actual workflow file to trigger this action. The README shows an example, but there's no actual workflow file in Here's What You Need to DoOption 1: Add the Missing Workflow File (Recommended)Create a new file: name: Bounty Command
on:
issue_comment:
types: [created]
jobs:
bounty-command:
# Only run when a top-level issue comment starts with /bounty
if: >
github.event.issue.pull_request == null &&
startsWith(github.event.comment.body, '/bounty')
runs-on: ubuntu-latest
# Add concurrency control to prevent race conditions
concurrency:
group: bounty-${{ github.event.issue.number }}
cancel-in-progress: false
steps:
- uses: actions/checkout@v4
- name: Parse /bounty amount from comment
id: parse
shell: bash
run: |
body=$(jq -r '.comment.body' "$GITHUB_EVENT_PATH")
# Expected formats: /bounty 25 or /bounty $25
raw="${body#/bounty }"
raw="${raw#\$}"
amount=$(echo "$raw" | awk '{print $1}')
echo "amount=$amount" >> "$GITHUB_OUTPUT"
- name: Call BLT bounty processor
uses: ./Bounty_Processor
with:
api_base_url: ${{ secrets.BLT_API_BASE_URL }}
blt_api_token: ${{ secrets.BLT_API_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}
owner: ${{ github.repository_owner }}
repo: ${{ github.event.repository.name }}
issue_number: ${{ github.event.issue.number }}
commenter: ${{ github.event.comment.user.login }}
issue_url: ${{ github.event.issue.html_url }}
amount: ${{ steps.parse.outputs.amount }}Note: I've added Option 2: Reorganize (Alternative)If you prefer to keep actions organized in
But this is only beneficial for internal organization. Since your README shows external users will reference this action, Option 1 is strongly recommended. Summary
The confusion likely arose because you have the composite action but not the workflow that uses it. Both are needed for the |
Companion PR in BLT:- OWASP-BLT/BLT#5273
Summary
This PR adds a composite GitHub Action and a repository workflow to:
/bountycommands in issue comments.$X Bountylabels accordingly.This allows maintainers or sponsors to manage BLT bounties directly from GitHub issues.
Changes
1. Composite action:
Bounty_Processor(action.yml)Name:
BLT Bounty ProcessorDescription: “Composite action that creates a bounty in BLT, reads total, updates labels, and posts a confirmation comment.”
Inputs:
api_base_url– Base URL for BLT API, e.g.https://<ngrok>.ngrok.io/api/v1blt_api_token–Token <value>used by BLT APIissue_number– GitHub issue number (string)issue_url– Full issue URLamount– Bounty amount (e.g.10or12.50)commenter– GitHub username of the bounty sponsorowner– Repository ownerrepo– Repository namegithub_token–GITHUB_TOKENfor GitHub API operationsSteps:
Validate amount
inputs.amountis:^[0-9]+(\.[0-9]{1,2})?$.amounttoGITHUB_OUTPUT.Create bounty via BLT API
Builds a JSON payload via an inline Python snippet:
{ "github_issue_url": "<issue_url>", "amount": "<amount>", "github_username": "<commenter>" }Sends:
Logs the raw JSON response (for debugging) and exposes it as
create_bounty.create_response.Query total bounty for issue from BLT API
URL-encodes
issue_urlvia a Python one-liner.Sends:
Parses the response JSON in Python, reading
total_amountwhen present and falling back toinputs.amountif anything fails.Exposes the resolved total as
steps.get_total.outputs.total.Update labels and post confirmation comment
actions/github-script@v7withgithub_token:Removes any existing labels matching:
Ensures the new label
$<total> Bountyexists (creates if needed, ignores already-exists errors).Adds that label to the issue.
Posts a confirmation comment, e.g.:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.