-
-
Notifications
You must be signed in to change notification settings - Fork 282
feature: auto-add GitHub issue badge with activity, views, and bounty #4897
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?
feature: auto-add GitHub issue badge with activity, views, and bounty #4897
Conversation
- Added badge creation on issue creation using BLT-Action - Display activity and view count for the past 30 days from IP logs - Show current bounty amount from GitHubIssues - Update issue description with badge image URL automatically
|
👋 Hi @kris70lesgo! This pull request needs a peer review before it can be merged. Please request a review from a team member who is not:
Once a valid peer review is submitted, this check will pass automatically. Thank you! |
WalkthroughAdds a GitHub Actions workflow to auto-insert an OWASP BLT activity badge into new issues and implements a new badge API (routing, view, SVG generation, caching/ETag, IP-based view logging, bounty lookup) plus tests exercising rendering, caching, and logging. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub
participant GHA as "GH Action"
participant API as "Badge API /api/v1/badge/issue/{id}/"
participant Cache
participant DB as "Database (GitHubIssue, IP)"
User->>GitHub: Open issue
GitHub->>GHA: Trigger workflow (issues: opened)
GHA->>API: GET /api/v1/badge/issue/{id}/
rect rgba(220,235,255,0.5)
Note over API,Cache: Cache-first with ETag (5 min)
API->>Cache: Lookup cached SVG + ETag
alt Cache hit & client ETag matches
Cache-->>API: cached SVG + ETag
API-->>GHA: 304 Not Modified
else Cache miss or ETag mismatch
API->>DB: Count distinct IPs (last 30 days)
DB-->>API: view_count
API->>DB: Fetch GitHubIssue.bounty
DB-->>API: bounty_amount
API->>API: generate_issue_badge_svg(view_count, bounty_amount)
API->>DB: Log client IP
API->>Cache: Store SVG + ETag (5 min)
API-->>GHA: 200 OK + SVG + ETag
end
end
GHA->>GitHub: Patch issue body to prepend badge Markdown if absent
GitHub-->>User: Issue updated (badge added)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Knowledge base: Disabled due to 📒 Files selected for processing (1)
⏰ 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). (3)
🔇 Additional comments (4)
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: 3
🧹 Nitpick comments (3)
blt/urls.py (1)
744-746: Sharing the^api/v1/prefix with router is fine, but be mindful of orderingHaving both
include(router.urls)andinclude("website.api.urls")under^api/v1/is valid and your badge path (badge/issue/...) won’t collide with the DRF router. Just keep this ordering in mind if you later add non‑router v1 endpoints to avoid accidental overlaps.website/api/views.py (2)
1044-1052: IP extraction helper is reasonable for metrics
get_client_iphandlesX-Forwarded-For,X-Real-IP, and falls back toREMOTE_ADDR, which is fine for non‑security metrics like view counting. Just keep in mind these headers are client‑/proxy‑controlled, so this must not be used for authentication or security decisions.
1055-1070: SVG generator matches requirements and is self‑contained
generate_issue_badge_svgcleanly encapsulates the badge markup, uses the BLT brand gradient, and keeps a compact footprint. Returning a stripped string is also good for caching/ETag consistency. No functional issues spotted.
📜 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 (5)
.github/workflows/auto-add-badge.yml(1 hunks)blt/urls.py(1 hunks)website/api/urls.py(1 hunks)website/api/views.py(2 hunks)website/tests/test_api.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.14.5)
website/api/views.py
1155-1155: Probable use of insecure hash functions in hashlib: md5
(S324)
1175-1175: Use explicit conversion flag
Replace with conversion flag
(RUF010)
website/tests/test_api.py
354-354: Local variable issue_no_bounty is assigned to but never used
Remove assignment to unused variable issue_no_bounty
(F841)
376-376: Local variable response is assigned to but never used
Remove assignment to unused variable response
(F841)
⏰ 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). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (1)
website/api/urls.py (1)
1-11: Badge API URL wiring looks correct
app_nameand thebadge/issue/<int:issue_number>/route line up with the tests and the GitHub Action URL (/api/v1/badge/issue/<issue_number>/). No issues here.
|
@sidd190 can u review the pr ? |
|
Hi @kris70lesgo. The approach seems alright to me. Kindly resolve the actionable comments posted by codeRabbit. Nitpicks pretty much seem fine for now. Also run pre-commit checks since one is failing due to an isort error. ( Also nothing major but just for convention, use fixes instead of closes in the PR description in general. It'll pass the check. ) Maybe drop a message here once this is done, and I'll review it again. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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 (1)
.github/workflows/auto-add-badge.yml (1)
20-20: Consider making the badge base URL configurable (optional refactor).The badge domain is hardcoded to
https://owaspblt.org(line 20). While this is acceptable for production, if you plan to support non-prod environments or testing contexts, consider driving this via a repository variable or environment configuration. A past review flagged this as "worth revisiting," but given the "Chill" review mode and MVP scope, this can be deferred to a follow-up if needed.For reference, the pattern would be:
steps: - name: Add activity badge to issue uses: actions/github-script@v7 + env: + BADGE_BASE_URL: ${{ vars.BADGE_BASE_URL || 'https://owaspblt.org' }} with: script: | const issue = context.payload.issue; - const badgeUrl = `https://owaspblt.org/api/v1/badge/issue/${issue.number}/`; + const badgeUrl = `${process.env.BADGE_BASE_URL}/api/v1/badge/issue/${issue.number}/`;Also applies to: 24-24
📜 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)
.github/workflows/auto-add-badge.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). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (1)
.github/workflows/auto-add-badge.yml (1)
1-48: Badge placement and workflow logic are correct.The workflow correctly prepends the badge section (line 30:
${badgeSection}${separator}${issue.body}), placing it at the top of the issue as intended. Duplicate detection is solid, null-body handling is sound, and error handling with logging is appropriate.
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
🧹 Nitpick comments (1)
website/api/views.py (1)
1176-1179: Use explicitexc_infoparameter in logger.The
exc_info=Trueparameter is already present, but static analysis suggests making it more explicit for clarity.This is a minor stylistic improvement. The current code is functionally correct.
- logger.error( - f"Error generating badge for issue {issue_number}: {str(e)}", - exc_info=True, - ) + logger.error( + "Error generating badge for issue %s: %s", + issue_number, + str(e), + exc_info=True, + )
📜 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)
website/api/views.py(2 hunks)website/tests/test_api.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.14.5)
website/api/views.py
1157-1157: Probable use of insecure hash functions in hashlib: md5
(S324)
1177-1177: Use explicit conversion flag
Replace with conversion flag
(RUF010)
⏰ 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). (3)
- GitHub Check: Run Tests
- GitHub Check: docker-test
- GitHub Check: docker-test
🔇 Additional comments (9)
website/api/views.py (5)
1-31: LGTM! Imports are appropriate for the badge functionality.The added imports support caching, HTTP responses, logging, and the badge API implementation.
1044-1052: LGTM! Client IP extraction correctly handles proxy scenarios.The function properly checks
X-Forwarded-For,X-Real-IP, andREMOTE_ADDRheaders in the correct priority order.
1055-1070: LGTM! SVG generation is clean and uses the correct brand colors.The template correctly incorporates the BLT brand red colors and displays both view count and bounty amount as specified.
1126-1136: Great fix! View count now uses the correct badge path.The view count aggregation now uses
path=badge_pathwhich matches the path being logged, addressing the previous review concern about path mismatch.
1157-1157: MD5 usage for ETag is acceptable here.While static analysis flags MD5 as insecure, this use case (generating ETags for cache validation) is not security-critical and MD5 is appropriate for fast, non-cryptographic hashing.
Based on learnings.
website/tests/test_api.py (4)
8-11: LGTM! Previous review concerns have been addressed.The unused variable assignments mentioned in the previous review have been fixed -
GitHubIssue.objects.create()on line 354 andself.client.get()on line 376 no longer assign to unused variables.
245-269: LGTM! Test setup provides appropriate fixtures.The setUp method creates a test repository and GitHub issue with a bounty, providing a solid foundation for the test cases.
282-299: Great fix! Test now uses the actual badge path.The test correctly seeds IP entries with
path="/api/v1/badge/issue/123/"which matches the path that the badge view logs, addressing the previous review concern about path mismatch between test data and actual implementation.
270-426: Excellent test coverage for the badge API.The test suite comprehensively covers:
- Endpoint accessibility and response types
- View count and bounty display
- Caching headers (Cache-Control, ETag)
- Conditional requests (304 responses)
- Non-existent issues and missing bounty cases
- IP logging and count increments
- SVG structure and brand colors
All 13 badge tests validate the key functionality described in the PR objectives.
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
📜 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)
website/api/views.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.14.5)
website/api/views.py
1148-1148: Probable use of insecure hash functions in hashlib: md5
(S324)
⏰ 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). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (7)
website/api/views.py (7)
1-38: LGTM! Imports are well-organized and necessary.All new imports support the badge functionality appropriately: hashlib for ETags, cache for response caching, HttpResponse for SVG delivery, and IP/GitHubIssue models for data retrieval.
1044-1052: LGTM! IP extraction follows best practices.The proxy-aware IP resolution correctly prioritizes
HTTP_X_FORWARDED_FOR, thenHTTP_X_REAL_IP, and finallyREMOTE_ADDR. This is appropriate for view-counting purposes.
1055-1070: LGTM! SVG generation is clean and secure.The badge structure is well-formed with proper styling and brand colors. The emoji usage (👁 💰) adds visual appeal, though note that emoji rendering can vary across systems.
1105-1113: Excellent fix! GitHubIssue lookup no longer creates records for arbitrary issue IDs.This addresses the previous concern about unbounded record creation. Using
.get()with a DoesNotExist handler that returns a default badge is the right approach.
1119-1127: Excellent fix! View count now uses exact badge path matching.This addresses the previous concern about path substring collisions. Using
path=badge_pathwith the exact badge URL ensures accurate view counting without cross-issue contamination.
1148-1148: MD5 usage is appropriate for ETags (static analysis false positive).The static analysis tool flags MD5 as insecure, but for ETag generation (cache validation), MD5 is perfectly acceptable and widely used. ETags don't require cryptographic security—only fast, consistent content hashing. This is standard practice for HTTP caching.
1166-1182: LGTM! Error handling ensures badge endpoint never breaks.The broad exception handler with fallback SVG is appropriate for badge endpoints—they should degrade gracefully rather than returning 500 errors that could break issue pages where the badge is embedded.
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
🧹 Nitpick comments (2)
website/api/views.py (2)
1044-1052: IP extraction helper looks fine; consider minor hardening.Logic is standard and works, but you may want to:
- Treat an empty/whitespace-only
HTTP_X_FORWARDED_FORas “missing” and fall back toHTTP_X_REAL_IP/REMOTE_ADDR.- Confirm at deployment level that
X-Forwarded-Foris only set by trusted proxies; otherwise it’s trivially spoofable.These are small robustness tweaks, not blockers.
1055-1070: SVG generator matches the badge requirements; only minor UX nits.The SVG generation is straightforward and uses the requested brand gradient and compact layout. Two minor, optional considerations:
- Emojis in SVG text aren’t guaranteed to render identically everywhere; if consistency matters, consider plain text labels or an option to disable emojis.
- If you ever localize or change copy length, a dynamic width based on text length would avoid truncation without editing the hard‑coded
width="300".No functional issues here.
📜 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)
website/api/views.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.14.5)
website/api/views.py
1148-1148: Probable use of insecure hash functions in hashlib: md5
(S324)
⏰ 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: docker-test
|
@sidd190 all the coderabbitai comments have been resolved and pre commit passed |
Issue Resolution
fixes #3917
Description
This PR implements an auto-generated SVG badge system for GitHub issues that displays real-time activity metrics and bounty information. The badge is automatically added to issue descriptions via a GitHub Actions workflow.
Features Implemented
1. Dynamic SVG Badge Generation
2. RESTful API Endpoint
GET /api/v1/badge/issue/<issue_id>/3. Automated GitHub Action Workflow
4. Performance Optimizations
Technical Implementation
Files Added
.github/workflows/auto-add-badge.yml— GitHub Action workflowwebsite/api/urls.py— API v1 URL routingFiles Modified
website/api/views.py— Badge endpoint implementationblt/urls.py— Include API URLswebsite/tests/test_api.py— 13 comprehensive test casesKey Functions
issue_badge_view()— Main endpoint handlergenerate_issue_badge_svg()— SVG template generationget_client_ip()— Client IP extraction with proxy supportURL Routing
Test Results
Endpoint accessibility
SVG format validation
View count accuracy
Bounty amount display
Cache headers verification
ETag conditional requests
IP logging functionality
Non-existent issue handling
Zero-bounty scenarios
Brand color compliance
Correct SVG structure
Full Test Suite: 108/109 Passing
108 tests passed (all relevant)
1 pre-existing Selenium error (Chrome setup, unrelated)
Notes
Summary by CodeRabbit
New Features
Tests