Add GitHub Discussions and Releases support - #103
Conversation
Adds comprehensive support for GitHub Discussions, including anonymous commenting via GraphQL API. Implements a releases viewer for both GitHub and GitLab repositories with asset downloads. Introduces server-side proxies for GitHub wiki and API endpoints to avoid CORS and rate-limiting issues. Enhances UI with discussion detail views, release information, and improved wiki navigation with anchor support and page caching.
📝 WalkthroughWalkthroughAdds GitHub Discussion proxying and anonymous comments, extends provider APIs, and introduces repository release browsing. The web interface also updates Discussion rendering, wiki navigation, markdown links, caching, and GitLab wiki visibility. ChangesGitHub Discussions backend
Repository web interface
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3920bbe5c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| api.GET("/gh-discussions/:owner/:repo", GitHubDiscussionsProxyHandler) | ||
| api.GET("/gh-discussion/:owner/:repo/:number", GitHubDiscussionDetailProxyHandler) |
There was a problem hiding this comment.
Gate public discussion proxies before using GITHUB_TOKEN
These new /api/gh-discussion(s) endpoints are under the unauthenticated public API group, while their handlers attach the server GITHUB_TOKEN to the GitHub GraphQL request. If that token can read any private repository, any caller can fetch private discussion titles, bodies, and comments just by naming owner/repo; add an access check such as verifying the repo is public, or avoid using the server token for this public proxy.
Useful? React with 👍 / 👎.
| const link = res.headers.get('Link') || ''; | ||
| const match = link.match(/&page=(\d+)>; rel="last"/); | ||
| const count = match ? parseInt(match[1]) : 0; |
There was a problem hiding this comment.
Count single-release repositories in the sidebar
When a GitHub repository has exactly one release, the per_page=1 response has no pagination Link header, so count becomes 0 even though releases[0] exists. That hides the Releases sidebar section for single-release projects, leaving the newly added release viewer undiscoverable in that common case; fall back to the returned array length when there is no rel="last" link.
Useful? React with 👍 / 👎.
| } | ||
| reportURL := fmt.Sprintf("%s://%s/v1/moderation/report?hash=%s", getScheme(c.Request), c.Request.Host, hash) | ||
|
|
||
| legend := fmt.Sprintf("\n\n---\ngoster-%s · karma (%d) · [report](%s)", hash, karma, reportURL) |
There was a problem hiding this comment.
Add discussion moderation before linking reports
For discussion comments this report link sends users into ReportHashHandler, but that moderation path only calls github.UpdateCommentsKarmaByHash/DeleteCommentsByHash, which search and mutate issue/PR REST comments under /issues/{n}/comments. Discussion comments created via GraphQL are never updated or deleted, so abuse reports on a discussion comment appear accepted while the reported comment stays live; add GraphQL discussion-comment moderation support before publishing the report link here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
internal/github/pr.go (2)
431-495: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGraphQL queries built via
%qstring interpolation instead of variables.
%qescapes using Go string-literal rules, not GraphQL/JSON string escaping. Control bytes in user-typedbody(e.g.\v,\a) get encoded into escape sequences GraphQL's parser doesn't recognize (\xHHisn't valid GraphQL), so some legitimate comment text will fail to post with an opaquegraphql: ...error. PassingdiscussionId/bodyas GraphQLvariables(JSON-encoded) sidesteps this entirely and is the standard way to safely interpolate user content into a GraphQL-over-HTTP request.♻️ Proposed fix using GraphQL variables
- mutation := fmt.Sprintf(`mutation { - addDiscussionComment(input: {discussionId: %q, body: %q}) { - comment { url } - } - }`, discussionID, body) - mutPayload := map[string]string{"query": mutation} + mutation := `mutation($discussionId: ID!, $body: String!) { + addDiscussionComment(input: {discussionId: $discussionId, body: $body}) { + comment { url } + } + }` + mutPayload := map[string]any{ + "query": mutation, + "variables": map[string]string{"discussionId": discussionID, "body": body}, + }The same fix applies to the id-resolution query (owner/repo/number via
variablesinstead of%q/%d).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/github/pr.go` around lines 431 - 495, Update CreateAnonymousDiscussionComment to stop interpolating owner, repo, discussion number, discussionID, and body into GraphQL strings with fmt.Sprintf; define GraphQL operations using variable placeholders and send the corresponding values through JSON-encoded variables in both the discussion ID query and addDiscussionComment mutation. Preserve the existing request, response decoding, and error handling behavior while ensuring all user-provided values are encoded by JSON.
1-1: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGraphQL queries hand-built via
%qstring interpolation across three sites — use GraphQLvariablesinstead.All three sites construct GraphQL request bodies by
fmt.Sprintf-ing values directly into the query text using%q/%d. Go's%qescaping (Go string-literal rules) doesn't match GraphQL/JSON string escaping (e.g.\xHH,\a,\varen't valid GraphQL escapes), so arbitrary text — especially the free-form commentbodyinpr.go— can produce a syntactically invalid GraphQL document and a confusing failure. The standard fix is to keep the query text static and pass values via a JSON-encodedvariablesmap, which guarantees correct escaping and removes the hand-rolled escaping surface entirely.
internal/github/pr.go#L431-495: passowner/repo/number(id-resolution query) anddiscussionId/body(mutation) as GraphQLvariablesinstead of interpolating with%q/%d.internal/http/handlers.go#L1509-1523: passowner/repoasvariablesin the discussions-list query instead of%qinterpolation.internal/http/handlers.go#L1626-1647: passowner/repo/numberasvariablesin the discussion-detail query instead of%q/%dinterpolation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/github/pr.go` at line 1, The GraphQL requests in the PR and HTTP handler flows interpolate values directly into query strings, causing invalid escaping and unsafe query construction. Update the id-resolution and comment mutation logic near the PR query sites, plus the discussions-list and discussion-detail handlers, to keep query text static and provide owner, repo, number, discussionId, and body through JSON-encoded GraphQL variables; remove the corresponding %q/%d interpolation while preserving existing request behavior.internal/http/handlers.go (1)
1509-1523: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSame GraphQL string-interpolation pattern as
internal/github/pr.go.
owner/repoare interpolated via%qinto the query text rather than passed as GraphQLvariables. Lower risk here than inpr.go(usernames/repo names are typically restricted character sets), but for consistency and defense-in-depth the samevariables-based fix should be applied. See consolidated comment.Also applies to: 1626-1647
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/http/handlers.go` around lines 1509 - 1523, Update the discussion GraphQL query and the corresponding query around the later discussion handler to use GraphQL variables for owner and repository instead of interpolating them with fmt.Sprintf. Define the variable declarations and references in the query, then pass owner and repo through the request’s variables payload, matching the variables-based pattern used in internal/github/pr.go.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/http/handlers.go`:
- Around line 1747-1785: Update GitHubWikiProxyHandler to URL-escape the owner,
repo, and page path segments before constructing pageURLs, ensuring characters
such as #, ?, &, and spaces remain part of the intended wiki path. Preserve the
existing .md fallback and request behavior.
- Around line 1966-2002: Move the karma and cooldown mutations from before the
provider call to after a successful prov.CreateAnonymousDiscussionComment in the
anonymous comment handler, keeping the existing reports-based karma calculation
and blocking checks unchanged. Apply the same ordering change to the
corresponding anonymous comment handlers that use updateKarma and
markFlaggedAction, so provider failures do not persist state.
In `@internal/http/router.go`:
- Around line 323-325: Update the route registrations for
GitHubDiscussionsProxyHandler, GitHubDiscussionDetailProxyHandler, and
GitHubWikiProxyHandler to apply a dedicated per-IP ghProxyLimiter, analogous to
prCheckLimiter, before invoking these public proxy handlers. Preserve the
existing route paths and middleware while ensuring unauthenticated clients
cannot exhaust the shared GitHub token budget; add short-TTL handler caching
only if needed to reduce redundant upstream requests.
In `@web/repo.html`:
- Around line 3583-3605: Update the GitHub releases logic around the count
calculation and the count > 0 && latest guard to fall back to releases.length
when the Link header does not provide a page count. Ensure repositories with one
release display the sidebar and latest release while preserving the existing
Link-header count for multi-release repositories.
- Around line 4118-4139: Sanitize the HTML produced by marked in renderMd before
it reaches any innerHTML assignment, using the existing DOMPurify integration or
an equivalent sanitizer. Apply this at every renderMd output sink for untrusted
issue, discussion, release, and wiki content, while preserving the existing
link-rendering behavior.
---
Nitpick comments:
In `@internal/github/pr.go`:
- Around line 431-495: Update CreateAnonymousDiscussionComment to stop
interpolating owner, repo, discussion number, discussionID, and body into
GraphQL strings with fmt.Sprintf; define GraphQL operations using variable
placeholders and send the corresponding values through JSON-encoded variables in
both the discussion ID query and addDiscussionComment mutation. Preserve the
existing request, response decoding, and error handling behavior while ensuring
all user-provided values are encoded by JSON.
- Line 1: The GraphQL requests in the PR and HTTP handler flows interpolate
values directly into query strings, causing invalid escaping and unsafe query
construction. Update the id-resolution and comment mutation logic near the PR
query sites, plus the discussions-list and discussion-detail handlers, to keep
query text static and provide owner, repo, number, discussionId, and body
through JSON-encoded GraphQL variables; remove the corresponding %q/%d
interpolation while preserving existing request behavior.
In `@internal/http/handlers.go`:
- Around line 1509-1523: Update the discussion GraphQL query and the
corresponding query around the later discussion handler to use GraphQL variables
for owner and repository instead of interpolating them with fmt.Sprintf. Define
the variable declarations and references in the query, then pass owner and repo
through the request’s variables payload, matching the variables-based pattern
used in internal/github/pr.go.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5447c3d-7f53-480f-bb91-dd3d8bba0a77
📒 Files selected for processing (7)
internal/github/pr.gointernal/http/handlers.gointernal/http/router.gointernal/provider/github/github.gointernal/provider/gitlab/gitlab.gointernal/provider/provider.goweb/repo.html
| func GitHubWikiProxyHandler(c *gin.Context) { | ||
| owner := c.Param("owner") | ||
| repo := c.Param("repo") | ||
| page := c.Param("page") | ||
| if page == "" { | ||
| page = "Home" | ||
| } | ||
|
|
||
| // intentar con .md primero, luego sin extension | ||
| pageURLs := []string{ | ||
| fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s.md", owner, repo, page), | ||
| fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s", owner, repo, page), | ||
| } | ||
|
|
||
| client := &http.Client{Timeout: 15 * time.Second} | ||
| for _, pageURL := range pageURLs { | ||
| req, err := http.NewRequest("GET", pageURL, nil) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| req.Header.Set("Accept", "text/plain") | ||
| req.Header.Set("User-Agent", "gitGost/1.0") | ||
|
|
||
| resp, err := client.Do(req) | ||
| if err != nil || resp.StatusCode != 200 { | ||
| if resp != nil { | ||
| resp.Body.Close() | ||
| } | ||
| continue | ||
| } | ||
| defer resp.Body.Close() | ||
| body, _ := io.ReadAll(resp.Body) | ||
| c.Data(200, "text/plain; charset=utf-8", body) | ||
| return | ||
| } | ||
|
|
||
| c.JSON(http.StatusNotFound, gin.H{"error": "wiki page not found"}) | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Wiki path segments aren't URL-escaped before being embedded into the target URL.
owner, repo, and especially page (a free-form wiki title) are inserted raw into the raw.githubusercontent.com URL. A page title containing #, ?, &, or spaces will truncate/corrupt the constructed URL (fragment/query boundary) or fail to match, silently returning a wrong page or 404 instead of the intended wiki page.
🔧 Proposed fix
pageURLs := []string{
- fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s.md", owner, repo, page),
- fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s", owner, repo, page),
+ fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s.md", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(page)),
+ fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(page)),
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func GitHubWikiProxyHandler(c *gin.Context) { | |
| owner := c.Param("owner") | |
| repo := c.Param("repo") | |
| page := c.Param("page") | |
| if page == "" { | |
| page = "Home" | |
| } | |
| // intentar con .md primero, luego sin extension | |
| pageURLs := []string{ | |
| fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s.md", owner, repo, page), | |
| fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s", owner, repo, page), | |
| } | |
| client := &http.Client{Timeout: 15 * time.Second} | |
| for _, pageURL := range pageURLs { | |
| req, err := http.NewRequest("GET", pageURL, nil) | |
| if err != nil { | |
| continue | |
| } | |
| req.Header.Set("Accept", "text/plain") | |
| req.Header.Set("User-Agent", "gitGost/1.0") | |
| resp, err := client.Do(req) | |
| if err != nil || resp.StatusCode != 200 { | |
| if resp != nil { | |
| resp.Body.Close() | |
| } | |
| continue | |
| } | |
| defer resp.Body.Close() | |
| body, _ := io.ReadAll(resp.Body) | |
| c.Data(200, "text/plain; charset=utf-8", body) | |
| return | |
| } | |
| c.JSON(http.StatusNotFound, gin.H{"error": "wiki page not found"}) | |
| } | |
| func GitHubWikiProxyHandler(c *gin.Context) { | |
| owner := c.Param("owner") | |
| repo := c.Param("repo") | |
| page := c.Param("page") | |
| if page == "" { | |
| page = "Home" | |
| } | |
| // intentar con .md primero, luego sin extension | |
| pageURLs := []string{ | |
| fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s.md", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(page)), | |
| fmt.Sprintf("https://raw.githubusercontent.com/wiki/%s/%s/%s", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(page)), | |
| } | |
| client := &http.Client{Timeout: 15 * time.Second} | |
| for _, pageURL := range pageURLs { | |
| req, err := http.NewRequest("GET", pageURL, nil) | |
| if err != nil { | |
| continue | |
| } | |
| req.Header.Set("Accept", "text/plain") | |
| req.Header.Set("User-Agent", "gitGost/1.0") | |
| resp, err := client.Do(req) | |
| if err != nil || resp.StatusCode != 200 { | |
| if resp != nil { | |
| resp.Body.Close() | |
| } | |
| continue | |
| } | |
| defer resp.Body.Close() | |
| body, _ := io.ReadAll(resp.Body) | |
| c.Data(200, "text/plain; charset=utf-8", body) | |
| return | |
| } | |
| c.JSON(http.StatusNotFound, gin.H{"error": "wiki page not found"}) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/http/handlers.go` around lines 1747 - 1785, Update
GitHubWikiProxyHandler to URL-escape the owner, repo, and page path segments
before constructing pageURLs, ensuring characters such as #, ?, &, and spaces
remain part of the intended wiki path. Preserve the existing .md fallback and
request behavior.
| userToken := req.UserToken | ||
| if strings.TrimSpace(userToken) == "" { | ||
| userToken = generateUserToken() | ||
| } | ||
| hash := deriveHash(owner, repo, number, userToken) | ||
| reports := getReportCountWithWindow(c.Request.Context(), hash) | ||
| if reports > 5 { | ||
| c.JSON(http.StatusForbidden, gin.H{"error": "hash bloqueado por reportes"}) | ||
| return | ||
| } | ||
| if reports > 2 { | ||
| if blocked := isFlaggedCooldown(hash); blocked { | ||
| c.JSON(http.StatusTooManyRequests, gin.H{"error": "cooldown activo por reportes"}) | ||
| return | ||
| } | ||
| } | ||
| currentKarma := getKarma(c.Request.Context(), hash) | ||
| karma := currentKarma + 1 | ||
| if reports > 2 { | ||
| karma = 0 | ||
| } | ||
| updateKarma(c.Request.Context(), hash, karma) | ||
| if reports > 2 { | ||
| markFlaggedAction(hash) | ||
| } | ||
| reportURL := fmt.Sprintf("%s://%s/v1/moderation/report?hash=%s", getScheme(c.Request), c.Request.Host, hash) | ||
|
|
||
| legend := fmt.Sprintf("\n\n---\ngoster-%s · karma (%d) · [report](%s)", hash, karma, reportURL) | ||
| bodyWithLegend := req.Body + legend | ||
|
|
||
| prov := providerFromPath(c.Request.URL.Path) | ||
| commentURL, err := prov.CreateAnonymousDiscussionComment(owner, repo, number, bodyWithLegend) | ||
| if err != nil { | ||
| utils.Log("Error creating discussion comment: %v", err) | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) | ||
| return | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check ordering of karma/report mutation vs provider call in sibling handlers
rg -n -B5 -A15 'CreateAnonymousComment\(|CreateAnonymousPRComment\(' internal/http/handlers.go | rg -n 'updateKarma|markFlaggedAction|CreateAnonymous(PR)?Comment\('Repository: livrasand/gitGost
Length of output: 342
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant handler blocks and the helper functions they call.
sed -n '1800,1865p' internal/http/handlers.go
printf '\n----\n'
sed -n '1885,1945p' internal/http/handlers.go
printf '\n----\n'
sed -n '1950,2015p' internal/http/handlers.go
printf '\n----\n'
rg -n 'func (updateKarma|markFlaggedAction|isFlaggedCooldown|getReportCountWithWindow|deriveHash|generateUserToken|providerFromPath)' internal/http/handlers.goRepository: livrasand/gitGost
Length of output: 6388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '2170,2265p' internal/http/handlers.goRepository: livrasand/gitGost
Length of output: 2178
Move karma/cooldown updates after the provider call.
updateKarma and markFlaggedAction run before prov.CreateAnonymousDiscussionComment, so a provider error still persists the new karma and can start cooldown from a failed attempt. The same ordering is used in the other anonymous comment handlers, so fix the shared pattern in one pass.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/http/handlers.go` around lines 1966 - 2002, Move the karma and
cooldown mutations from before the provider call to after a successful
prov.CreateAnonymousDiscussionComment in the anonymous comment handler, keeping
the existing reports-based karma calculation and blocking checks unchanged.
Apply the same ordering change to the corresponding anonymous comment handlers
that use updateKarma and markFlaggedAction, so provider failures do not persist
state.
| api.GET("/gh-discussions/:owner/:repo", GitHubDiscussionsProxyHandler) | ||
| api.GET("/gh-discussion/:owner/:repo/:number", GitHubDiscussionDetailProxyHandler) | ||
| api.GET("/gh-wiki/:owner/:repo/:page", GitHubWikiProxyHandler) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
New public /api proxy routes have no rate limiting, unlike sibling routes in this file.
gh-discussions, gh-discussion, and gh-wiki (handlers in internal/http/handlers.go) are registered here with no middleware beyond the global securityHeaders/localhostCORS — no sizeLimitMiddleware, no per-IP limiter comparable to prCheckLimiter() (line 313) or adminLimiter() (line 336). The first two proxy to GitHub's GraphQL endpoint using the server's shared GITHUB_TOKEN, whose GraphQL point budget (5000/hr) is easy to exhaust from a single unauthenticated client hammering this route, degrading every other feature relying on the same token (anonymous issue/PR/discussion comments, badges). This runs counter to the PR's stated goal of addressing rate-limiting issues.
🛡️ Suggested direction
- api.GET("/gh-discussions/:owner/:repo", GitHubDiscussionsProxyHandler)
- api.GET("/gh-discussion/:owner/:repo/:number", GitHubDiscussionDetailProxyHandler)
- api.GET("/gh-wiki/:owner/:repo/:page", GitHubWikiProxyHandler)
+ api.GET("/gh-discussions/:owner/:repo", ghProxyLimiter(), GitHubDiscussionsProxyHandler)
+ api.GET("/gh-discussion/:owner/:repo/:number", ghProxyLimiter(), GitHubDiscussionDetailProxyHandler)
+ api.GET("/gh-wiki/:owner/:repo/:page", ghProxyLimiter(), GitHubWikiProxyHandler)(ghProxyLimiter being a new limiter analogous to prCheckLimiter, plus optional short-TTL caching in the handlers to further cut redundant upstream calls.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/http/router.go` around lines 323 - 325, Update the route
registrations for GitHubDiscussionsProxyHandler,
GitHubDiscussionDetailProxyHandler, and GitHubWikiProxyHandler to apply a
dedicated per-IP ghProxyLimiter, analogous to prCheckLimiter, before invoking
these public proxy handlers. Preserve the existing route paths and middleware
while ensuring unauthenticated clients cannot exhaust the shared GitHub token
budget; add short-TTL handler caching only if needed to reduce redundant
upstream requests.
| const res = await ghFetch(`https://api.github.com/repos/${rv.owner}/${rv.repo}/releases?per_page=1`); | ||
| if (!res.ok) { el.textContent = ''; if (section) section.style.display = 'none'; return; } | ||
| const link = res.headers.get('Link') || ''; | ||
| const match = link.match(/&page=(\d+)>; rel="last"/); | ||
| const count = match ? parseInt(match[1]) : 0; | ||
| const releases = await res.json(); | ||
| const latest = releases[0]; | ||
| if (count > 0 && latest) { | ||
| el.textContent = count.toLocaleString(); | ||
| if (latestEl) { | ||
| const tag = latest.tag_name || latest.name || ''; | ||
| const name = latest.name || tag; | ||
| const ago = timeAgo(latest.published_at || latest.created_at); | ||
| latestEl.innerHTML = `<a href="#" onclick="showView('releases'); return false;" style="color:var(--fg);text-decoration:none;display:block;padding:0.3rem 0;"> | ||
| <div style="font-size:0.82rem;font-weight:500;line-height:1.3;">${escHtml(name)} <span class="release-latest-badge">Latest</span></div> | ||
| ${ago ? `<div style="font-size:0.7rem;color:var(--fg-muted);margin-top:0.15rem;">${ago}</div>` : ''} | ||
| </a>`; | ||
| } | ||
| if (section) section.style.display = ''; | ||
| } else { | ||
| el.textContent = ''; | ||
| if (section) section.style.display = 'none'; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Single-release GitHub repos hide the Releases sidebar.
With per_page=1, GitHub omits the Link header when there is only one release, so count stays 0 and the count > 0 && latest guard hides the section even though a release exists. The GitLab branch already falls back to data.length; mirror that here.
🐛 Proposed fix
const link = res.headers.get('Link') || '';
const match = link.match(/&page=(\d+)>; rel="last"/);
- const count = match ? parseInt(match[1]) : 0;
const releases = await res.json();
+ const count = match ? parseInt(match[1]) : releases.length;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const res = await ghFetch(`https://api.github.com/repos/${rv.owner}/${rv.repo}/releases?per_page=1`); | |
| if (!res.ok) { el.textContent = ''; if (section) section.style.display = 'none'; return; } | |
| const link = res.headers.get('Link') || ''; | |
| const match = link.match(/&page=(\d+)>; rel="last"/); | |
| const count = match ? parseInt(match[1]) : 0; | |
| const releases = await res.json(); | |
| const latest = releases[0]; | |
| if (count > 0 && latest) { | |
| el.textContent = count.toLocaleString(); | |
| if (latestEl) { | |
| const tag = latest.tag_name || latest.name || ''; | |
| const name = latest.name || tag; | |
| const ago = timeAgo(latest.published_at || latest.created_at); | |
| latestEl.innerHTML = `<a href="#" onclick="showView('releases'); return false;" style="color:var(--fg);text-decoration:none;display:block;padding:0.3rem 0;"> | |
| <div style="font-size:0.82rem;font-weight:500;line-height:1.3;">${escHtml(name)} <span class="release-latest-badge">Latest</span></div> | |
| ${ago ? `<div style="font-size:0.7rem;color:var(--fg-muted);margin-top:0.15rem;">${ago}</div>` : ''} | |
| </a>`; | |
| } | |
| if (section) section.style.display = ''; | |
| } else { | |
| el.textContent = ''; | |
| if (section) section.style.display = 'none'; | |
| } | |
| const res = await ghFetch(`https://api.github.com/repos/${rv.owner}/${rv.repo}/releases?per_page=1`); | |
| if (!res.ok) { el.textContent = ''; if (section) section.style.display = 'none'; return; } | |
| const link = res.headers.get('Link') || ''; | |
| const match = link.match(/&page=(\d+)>; rel="last"/); | |
| const releases = await res.json(); | |
| const count = match ? parseInt(match[1]) : releases.length; | |
| const latest = releases[0]; | |
| if (count > 0 && latest) { | |
| el.textContent = count.toLocaleString(); | |
| if (latestEl) { | |
| const tag = latest.tag_name || latest.name || ''; | |
| const name = latest.name || tag; | |
| const ago = timeAgo(latest.published_at || latest.created_at); | |
| latestEl.innerHTML = `<a href="#" onclick="showView('releases'); return false;" style="color:var(--fg);text-decoration:none;display:block;padding:0.3rem 0;"> | |
| <div style="font-size:0.82rem;font-weight:500;line-height:1.3;">${escHtml(name)} <span class="release-latest-badge">Latest</span></div> | |
| ${ago ? `<div style="font-size:0.7rem;color:var(--fg-muted);margin-top:0.15rem;">${ago}</div>` : ''} | |
| </a>`; | |
| } | |
| if (section) section.style.display = ''; | |
| } else { | |
| el.textContent = ''; | |
| if (section) section.style.display = 'none'; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 3595-3598: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: latestEl.innerHTML = <a href="#" onclick="showView('releases'); return false;" style="color:var(--fg);text-decoration:none;display:block;padding:0.3rem 0;"> <div style="font-size:0.82rem;font-weight:500;line-height:1.3;">${escHtml(name)} <span class="release-latest-badge">Latest</span></div> ${ago ?
: ''} </a>Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/repo.html` around lines 3583 - 3605, Update the GitHub releases logic
around the count calculation and the count > 0 && latest guard to fall back to
releases.length when the Link header does not provide a page count. Ensure
repositories with one release display the sidebar and latest release while
preserving the existing Link-header count for multi-release repositories.
| function renderMd(md, baseUrl, isWiki) { | ||
| if (typeof marked === 'undefined') return `<pre>${escHtml(md)}</pre>`; | ||
| marked.setOptions({ gfm: true, breaks: false }); | ||
| marked.setOptions({ gfm: true, breaks: false, headerIds: true, headerPrefix: '' }); | ||
| const renderer = new marked.Renderer(); | ||
| // marked v4: link token es un objeto {href, title, text} | ||
| renderer.link = function(token) { | ||
| const href = (token && token.href != null) ? token.href : (arguments[0] || ''); | ||
| const title = (token && token.title != null) ? token.title : (arguments[1] || null); | ||
| const text = (token && token.text != null) ? token.text : (arguments[2] || ''); | ||
| return `<a href="${href}" target="_blank" rel="noopener"${title ? ` title="${title}"` : ''}>${text}</a>`; | ||
| const t = title ? ` title="${escAttr(title)}"` : ''; | ||
| if (typeof href === 'string' && href.startsWith('#')) { | ||
| const anchor = href.slice(1); | ||
| return anchor ? `<a href="#" data-anchor="${escAttr(anchor)}"${t}>${text}</a>` : `<a href="#"${t}>${text}</a>`; | ||
| } | ||
| if (isWiki && typeof href === 'string' && !/^[a-z][a-z0-9+.-]*:/i.test(href) && !/^\/\//.test(href)) { | ||
| const p = parseWikiHref(href); | ||
| if (p.slug || p.anchor) { | ||
| const target = p.slug + (p.anchor ? '#' + p.anchor : ''); | ||
| return `<a href="#" data-wiki="${escAttr(target)}"${t}>${text}</a>`; | ||
| } | ||
| } | ||
| return `<a href="${escAttr(href)}" target="_blank" rel="noopener"${t}>${text}</a>`; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
marked v4 does it sanitize raw HTML in markdown by default
💡 Result:
No, Marked does not sanitize raw HTML by default in any version, including v4 [1][2][3]. Marked is a markdown parser and does not perform security filtering on its output [1][2]. If you process untrusted markdown input that may contain raw HTML, you are responsible for sanitizing the resulting HTML to prevent Cross-Site Scripting (XSS) attacks [1][4]. The official documentation consistently recommends using a dedicated sanitization library such as DOMPurify, sanitize-html, or insane on the output generated by Marked [2][4]. For example, you can integrate sanitization using a post-process hook in Marked: import { marked } from 'marked'; import DOMPurify from 'dompurify'; marked.use({ hooks: { postprocess(html) { return DOMPurify.sanitize(html); } } }); const html = marked.parse(userMarkdown); Older versions of Marked included a 'sanitize' option, but this was removed in v8.0.0 (and was already deprecated long before that) [5][6]. Regardless of the version, Marked's architecture has historically prioritized performance and extensibility over built-in security sanitization [1][7].
Citations:
- 1: https://marked.js.org/
- 2: https://github.com/markedjs/marked?sign=iDjZoVsPXx3TyQpnnjcpQPYUaDST6Qc2biXiBe3VVBo&ts=1772024115
- 3: https://github.com/markedjs/marked?sign=JdKx77WOenkFhnzCYZD2WFgRjg7BCHJjXijzPDFo6sk&ts=1774642191
- 4: https://github.com/markedjs/marked/blob/v18.0.0/docs/INDEX.md
- 5: https://marked.js.org/using_advanced
- 6: https://github.com/markedjs/marked/blob/master/docs/USING_ADVANCED.md
- 7: Add optional
sanitizeroption for easier secure rendering with DOMPurify markedjs/marked#3943
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and locate the renderer + sinks.
ast-grep outline web/repo.html --view expanded || true
printf '\n--- renderMd occurrences ---\n'
rg -n "renderMd\s*\(" web/repo.html || true
printf '\n--- innerHTML assignments mentioning renderMd ---\n'
rg -n "innerHTML\s*=.*renderMd|renderMd.*innerHTML" web/repo.html || true
printf '\n--- relevant surrounding lines around renderMd ---\n'
line=$(rg -n "function renderMd\(md, baseUrl, isWiki\)" web/repo.html | head -n1 | cut -d: -f1)
if [ -n "${line:-}" ]; then
start=$((line-20))
end=$((line+140))
sed -n "${start},${end}p" web/repo.html | cat -n
fiRepository: livrasand/gitGost
Length of output: 12435
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate all places that convert markdown to HTML and write to DOM.
rg -n "innerHTML\s*=|insertAdjacentHTML|outerHTML\s*=|renderMd\(" web/repo.html || true
printf '\n--- nearby source selection logic ---\n'
rg -n "discussion|release note|release notes|comment|body|markdown|md" web/repo.html | head -n 80 || trueRepository: livrasand/gitGost
Length of output: 15881
Sanitize markdown output before assigning it to innerHTML.
renderMd uses marked.parse() and the result is injected into multiple DOM sinks without sanitization. Since the content comes from untrusted issue/discussion/release/wiki bodies, raw HTML in markdown (for example, <img onerror=...>) can execute. Pass the rendered HTML through DOMPurify or a similar sanitizer at each sink.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/repo.html` around lines 4118 - 4139, Sanitize the HTML produced by marked
in renderMd before it reaches any innerHTML assignment, using the existing
DOMPurify integration or an equivalent sanitizer. Apply this at every renderMd
output sink for untrusted issue, discussion, release, and wiki content, while
preserving the existing link-rendering behavior.
Source: Linters/SAST tools
Adds comprehensive support for GitHub Discussions, including anonymous commenting via GraphQL API. Implements a releases viewer for both GitHub and GitLab repositories with asset downloads. Introduces server-side proxies for GitHub wiki and API endpoints to avoid CORS and rate-limiting issues. Enhances UI with discussion detail views, release information, and improved wiki navigation with anchor support and page caching.
Summary by CodeRabbit