Add GitLab commit APIs & commits UI - #100
Conversation
Introduce GitLab proxy endpoints and a full commits UI. Server: add GitLab handlers (commit count with X-Total or binary-search fallback, avatar-by-email, commits list, commit detail that merges info + diff). Register routes and update CSP to allow esm.sh for client-side modules. Client/web: add commits view and commit detail UI, responsive styles for index/repo pages, Shiki-based async diff highlighting (fallback to multiple CDNs), gravatar/md5 helper, avatar prefetch via new API, and resolve relative image/link paths in rendered markdown. Also call loadCommitCount to populate sidebar. Minor icon and layout tweaks.
📝 WalkthroughWalkthroughGitLab proxy endpoints and repository commit browsing were added, including commit counts, details, avatars, diffs, and syntax highlighting. Markdown relative links now resolve against repository content URLs, while responsive layouts and content security policies were updated. ChangesGitLab repository integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RepositoryUI
participant GitGostAPI
participant GitLab
RepositoryUI->>GitGostAPI: Request commits, avatar, count, or detail
GitGostAPI->>GitLab: Fetch project data
GitLab-->>GitGostAPI: Return commit response
GitGostAPI-->>RepositoryUI: Return JSON data
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62bd1fe468
ℹ️ 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".
| avatarHtml = `<img class="cd-avatar" src="${avatarUrl}" alt="${author}" />`; | ||
| } else if (email) { | ||
| const gravatarSrc = `https://www.gravatar.com/avatar/${md5(email.trim().toLowerCase())}?s=28&d=identicon`; | ||
| avatarHtml = `<img class="cd-avatar" src="${gravatarSrc}" alt="${author}" onerror='this.outerHTML="<div class=cd-avatar style=\\"display:flex;align-items:center;justify-content:center;background:var(--bg-hover);color:var(--fg-muted);font-size:0.7rem;font-weight:600;\">${initial}</div>"' />`; |
There was a problem hiding this comment.
When a repository contains a commit whose author name includes a quote and an inline handler, this interpolates that untrusted name directly into the <img> attributes; for example a name like " onload="alert(1) breaks out of alt and executes when the avatar loads, and the current CSP still allows inline handlers. Please attribute-escape or build this element with DOM APIs before rendering commit details.
Useful? React with 👍 / 👎.
| function str2binl(s){let bin=Array(s.length>>2);for(let i=0;i<bin.length;i++)bin[i]=0;for(let i=0;i<s.length*8;i+=8)bin[i>>5]|=(s.charCodeAt(i/8)&255)<<(i%32);return bin} | ||
| function coreMD5(s){let bin=str2binl(s);bin[s.length*8>>5]|=0x80<<((s.length*8)%32);bin[(((s.length*8+64)>>9)<<4)+14]=s.length*8;let h=[0x67452301,0xEFCDAB89,0x98BADCFE,0x10325476];for(let i=0;i<bin.length;i+=16)md5cycle(h,bin.slice(i,i+16));return h} |
There was a problem hiding this comment.
For normal email inputs this helper produces non-standard MD5 digests because the padded block passed to md5cycle is sparse, so missing words become undefined and collapse arithmetic inside add32; for example test@example.com becomes 963d7832f10838429ba51eb95823fc59 instead of Gravatar's required 55502f40dc8b7c769880b10874abc9d0. As a result, users with Gravatar avatars are shown unrelated/default identicons in the new commits UI.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
web/index.html (1)
1049-1099: 📐 Maintainability & Code Quality | 🔵 TrivialAttribute selectors matching inline styles are brittle and will break silently.
These
#page-content [style*="..."]selectors depend on exact substring matches of inline style strings. If any inline style's formatting changes (whitespace, property order, value precision), the override silently stops applying with no error or warning. This is a maintainability risk that will cause subtle mobile regressions over time.Consider extracting the inline styles into CSS classes and overriding those classes in the media query instead. This decouples the responsive overrides from the exact inline style format.
[medium_effort_and_high_reward]
🤖 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/index.html` around lines 1049 - 1099, Replace the brittle `#page-content` [style*="..."] responsive selectors with semantic CSS classes applied to the corresponding elements. Define those classes in the base styles and target the classes within the mobile media query, preserving the existing responsive behavior without depending on inline-style formatting.web/repo.html (1)
2366-2372: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGitLab per-file additions/deletions are always 0.
additions/deletionsare hardcoded to0, so the+N/-Nbadges in the diff file header (Lines 2510-2511) never appear for GitLab commits. GitLab's diff endpoint doesn't return counts, but they can be derived by counting+/-lines ind.diff.♻️ Derive counts from the patch
- files = rawDiff.map(d => ({ - filename: d.new_path, - status: d.new_file ? 'added' : d.deleted_file ? 'removed' : d.renamed_file ? 'renamed' : 'modified', - additions: 0, - deletions: 0, - patch: d.diff || '' - })); + files = rawDiff.map(d => { + const diff = d.diff || ''; + const lines = diff.split('\n'); + return { + filename: d.new_path, + status: d.new_file ? 'added' : d.deleted_file ? 'removed' : d.renamed_file ? 'renamed' : 'modified', + additions: lines.filter(l => l.startsWith('+') && !l.startsWith('+++')).length, + deletions: lines.filter(l => l.startsWith('-') && !l.startsWith('---')).length, + patch: diff + }; + });🤖 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 2366 - 2372, In the GitLab `rawDiff.map` transformation, replace the hardcoded `additions` and `deletions` values with counts derived from `d.diff`: count added lines beginning with `+` (excluding the `+++` file header) and removed lines beginning with `-` (excluding `---`), then assign those counts to the corresponding fields so the diff badges render correctly.
🤖 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 1492-1498: Update the diff response handling in the relevant
handler to detect and represent GitLab’s truncation indicators instead of
assuming a successful JSON payload is complete. Ensure commitData["diff_files"]
is either populated from a source that provides the full file list or includes
an explicit truncation status/metadata so the UI can handle incomplete results.
- Around line 1424-1428: Update the SHA validation in the handler containing the
existing length check to also reject any non-hexadecimal characters before
proxying to GitLab. Keep the 6–64 character bounds and add a hex-only validation
using an appropriate standard-library check or equivalent, returning the same
bad-request response for invalid values.
In `@internal/http/router.go`:
- Line 57: Remove the insecure `http://*` source from the `img-src` directive in
the router’s CSP configuration, and replace the broad `https://*` allowance with
only the specific trusted image hosts required by the application, such as
configured CDNs, GitLab, or Gravatar.
In `@web/index.html`:
- Around line 1058-1063: Replace word-break: break-all in the `#page-content`
code[style*="display:block"] rule with overflow-wrap: break-word (or anywhere)
so long content can wrap without splitting normal words unnecessarily.
In `@web/repo.html`:
- Line 2537: In the file-count display, replace the use of stats.total with the
files array length for both the rendered count and singular/plural selection.
Update the expression in the relevant template span so commits always show
files.length files, with appropriate handling when files is unavailable.
---
Nitpick comments:
In `@web/index.html`:
- Around line 1049-1099: Replace the brittle `#page-content` [style*="..."]
responsive selectors with semantic CSS classes applied to the corresponding
elements. Define those classes in the base styles and target the classes within
the mobile media query, preserving the existing responsive behavior without
depending on inline-style formatting.
In `@web/repo.html`:
- Around line 2366-2372: In the GitLab `rawDiff.map` transformation, replace the
hardcoded `additions` and `deletions` values with counts derived from `d.diff`:
count added lines beginning with `+` (excluding the `+++` file header) and
removed lines beginning with `-` (excluding `---`), then assign those counts to
the corresponding fields so the diff badges render correctly.
🪄 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: 63fe90ed-c289-4925-8247-c338322d5d3b
📒 Files selected for processing (4)
internal/http/handlers.gointernal/http/router.goweb/index.htmlweb/repo.html
| // Validar que sha sea un hash hexadecimal valido | ||
| if len(sha) < 6 || len(sha) > 64 { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "invalid sha"}) | ||
| return | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm Gin URL-decodes path params (so ?,&,# can appear in c.Param("sha"))
rg -nP 'c\.Param\(' -C2 internal/http/handlers.go | head -40Repository: livrasand/gitGost
Length of output: 1268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant handler and route registration around it.
sed -n '1390,1495p' internal/http/handlers.go
printf '\n--- ROUTES ---\n'
rg -n 'sha|GitLab.*Compare|Compare.*Handler|/compare' internal/http/handlers.goRepository: livrasand/gitGost
Length of output: 9158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the target function and any route patterns that bind `:sha`.
sed -n '1410,1485p' internal/http/handlers.go
printf '\n---\n'
rg -n ':sha|sha' internal/http/handlers.goRepository: livrasand/gitGost
Length of output: 8159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the specific compare handler body and surrounding route definition.
awk 'NR>=1418 && NR<=1478 {printf "%4d %s\n", NR, $0}' internal/http/handlers.go
printf '\n--- ROUTE MATCHES ---\n'
rg -n 'Compare|compare|:sha' internal/http/handlers.goRepository: livrasand/gitGost
Length of output: 2218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "invalid sha|sha.*hex|isHexString|private_token|repository/compare|compare" internal/http/handlers.goRepository: livrasand/gitGost
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "GitLabCommitDetailHandler|:sha|repository/.*/commits|commits/:sha|/commits/" .Repository: livrasand/gitGost
Length of output: 1196
🌐 Web query:
Gin Context.Param URL-decodes path parameters percent-encoded characters path segment documentation
💡 Result:
In the Gin web framework, the behavior of c.Param regarding URL-decoded path parameters depends on the framework's configuration, particularly the settings for UseRawPath and UnescapePathValues [1][2][3]. By default, the path is typically decoded before reaching the routing engine because the underlying net/http library decodes URL.Path [2]. Consequently, when you access a path parameter via c.Param, it generally returns the already-decoded value [1][2]. However, if you enable UseRawPath (e.g., set to true), Gin attempts to use the raw, encoded URL path for routing [1][3]. When used in conjunction with UnescapePathValues=true, Gin will automatically attempt to unescape the path parameter values [2][3]. Regarding the specific decoding mechanism: 1. Historically, Gin used url.QueryUnescape to decode these parameters [2]. 2. Recent updates (as of mid-2026) have moved to using url.PathUnescape for path parameters to better align with RFC 3986 standards, specifically ensuring that literal plus signs (+) in path segments are preserved rather than converted to spaces (a behavior associated with application/x-www-form-urlencoded) [2]. If you encounter issues where specific percent-encoded characters (like %2F for a forward slash) are causing routing errors, it is often due to the path being decoded before the router can properly distinguish between path segments [1][4]. Enabling UseRawPath can help the router handle these characters correctly, though it requires careful management of UnescapePathValues to ensure the parameters are processed as desired in your application logic [1][3].
Citations:
- 1: Path decoded before routing gin-gonic/gin#3024
- 2: fix(tree): use url.PathUnescape for path parameters gin-gonic/gin#4674
- 3: Unexpected c.Param() results when UseRawPath and UnescapePathValues are true gin-gonic/gin#2633
- 4: https://stackoverflow.com/questions/71581828/gin-problem-accessing-url-encoded-path-param-containing-forward-slash
Reject non-hex sha values before proxying to GitLab. Length-only validation lets invalid refs through; add a hex-only check here.
🤖 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 1424 - 1428, Update the SHA
validation in the handler containing the existing length check to also reject
any non-hexadecimal characters before proxying to GitLab. Keep the 6–64
character bounds and add a hex-only validation using an appropriate
standard-library check or equivalent, returning the same bad-request response
for invalid values.
| // Add diff files to the response | ||
| if diffRes.ok && diffRes.data != nil { | ||
| var diffData []interface{} | ||
| if err := json.Unmarshal(diffRes.data, &diffData); err == nil { | ||
| commitData["diff_files"] = diffData | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
GitLab API repository commit diff endpoint pagination per_page default
💡 Result:
The GitLab API endpoint for retrieving a commit diff (GET /projects/:id/repository/commits/:sha/diff) does not support pagination parameters such as per_page or page [1][2][3][4]. Unlike list-based endpoints in the GitLab API (which often default to 20 results per page) [5], the commit diff endpoint returns the diff data for the specific commit requested [3][4]. It is explicitly documented that this endpoint is subject to diff limits [3][4]. If a commit exceeds the configured maximum number of files or size, the response will be truncated, and no additional files are returned beyond those limits [3][4]. Because it is not a paginated endpoint, the per_page parameter has no effect on the number of results returned [3][6][4].
Citations:
- 1: https://docs.gitlab.com/api/commits/
- 2: https://docs.gitlab.com/17.11/api/commits/
- 3: https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/api/commits.md?ref_type=heads
- 4: https://github.com/gitlabhq/gitlabhq/blob/master/doc/api/commits.md
- 5: https://docs.gitlab.com/api/repositories/
- 6: https://forum.gitlab.com/t/commits-diff-api-cannot-get-more-than-100-results/87916
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the surrounding code and the diffRes call site to see how diff_files is populated.
grep -n "diffRes" -n internal/http/handlers.go | head -20
sed -n '1460,1510p' internal/http/handlers.goRepository: livrasand/gitGost
Length of output: 1758
GitLab’s /diff response can still be truncated for large commits
GitLab doesn’t paginate this endpoint, but it does enforce diff limits, so diff_files may be incomplete for big commits. If the UI needs the full file list, handle truncation explicitly or use another source.
🤖 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 1492 - 1498, Update the diff response
handling in the relevant handler to detect and represent GitLab’s truncation
indicators instead of assuming a successful JSON payload is complete. Ensure
commitData["diff_files"] is either populated from a source that provides the
full file list or includes an explicit truncation status/metadata so the UI can
handle incomplete results.
| "style-src 'self' https://fonts.googleapis.com https://cdnjs.cloudflare.com 'unsafe-inline'; "+ | ||
| "font-src 'self' https://fonts.gstatic.com; "+ | ||
| "img-src 'self' data: blob: https://*.amazonaws.com https://*.s3.amazonaws.com https://cdn.simpleicons.org https://img.shields.io https://trendshift.io https://api.star-history.com; "+ | ||
| "img-src 'self' data: blob: https://* http://*; "+ |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
img-src http://* weakens CSP by permitting insecure/mixed-content images from any origin.
Widening img-src to https://* http://* removes the domain allowlist entirely. The http://* entry in particular permits plaintext image loads from arbitrary hosts (mixed content). If arbitrary avatar/Gravatar URLs are needed, at minimum drop http://* and keep only https://*, or restrict to the specific hosts actually used (e.g. gitlab.com, gravatar, the configured CDNs).
🛡️ Suggested tightening
- "img-src 'self' data: blob: https://* http://*; "+
+ "img-src 'self' data: blob: https://*; "+📝 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.
| "img-src 'self' data: blob: https://* http://*; "+ | |
| "img-src 'self' data: blob: https://*; "+ |
🤖 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` at line 57, Remove the insecure `http://*` source
from the `img-src` directive in the router’s CSP configuration, and replace the
broad `https://*` allowance with only the specific trusted image hosts required
by the application, such as configured CDNs, GitLab, or Gravatar.
| #page-content code[style*="display:block"] { | ||
| font-size: 0.72rem !important; | ||
| padding: 0.4rem 0.5rem !important; | ||
| white-space: normal !important; | ||
| word-break: break-all !important; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
word-break: break-all is overly aggressive for code blocks.
break-all breaks at any character, including mid-word in normal text, making content harder to read. overflow-wrap: break-word (or anywhere) only breaks when content would otherwise overflow, preserving readability for shorter strings.
♻️ Proposed fix
`#page-content` code[style*="display:block"] {
font-size: 0.72rem !important;
padding: 0.4rem 0.5rem !important;
white-space: normal !important;
- word-break: break-all !important;
+ overflow-wrap: break-word !important;
}📝 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.
| #page-content code[style*="display:block"] { | |
| font-size: 0.72rem !important; | |
| padding: 0.4rem 0.5rem !important; | |
| white-space: normal !important; | |
| word-break: break-all !important; | |
| } | |
| `#page-content` code[style*="display:block"] { | |
| font-size: 0.72rem !important; | |
| padding: 0.4rem 0.5rem !important; | |
| white-space: normal !important; | |
| overflow-wrap: break-word !important; | |
| } |
🤖 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/index.html` around lines 1058 - 1063, Replace word-break: break-all in
the `#page-content` code[style*="display:block"] rule with overflow-wrap:
break-word (or anywhere) so long content can wrap without splitting normal words
unnecessarily.
| <span class="cd-stats"> | ||
| <span class="add">+${stats.additions || 0}</span> | ||
| <span class="del">-${stats.deletions || 0}</span> | ||
| <span style="color:var(--fg-muted);">${stats.total || (files ? files.length : 0)} ${((stats.total || files?.length) === 1) ? 'file' : 'files'}</span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Incorrect file count: stats.total is line changes, not number of files.
For GitHub, commit.stats.total equals additions + deletions, so a commit touching 3 files with 50 changed lines renders "50 files". Both the count and the pluralization should be driven by files.length.
🐛 Fix count and pluralization
- <span style="color:var(--fg-muted);">${stats.total || (files ? files.length : 0)} ${((stats.total || files?.length) === 1) ? 'file' : 'files'}</span>
+ <span style="color:var(--fg-muted);">${files ? files.length : 0} ${(files?.length === 1) ? 'file' : 'files'}</span>📝 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.
| <span style="color:var(--fg-muted);">${stats.total || (files ? files.length : 0)} ${((stats.total || files?.length) === 1) ? 'file' : 'files'}</span> | |
| <span style="color:var(--fg-muted);">${files ? files.length : 0} ${(files?.length === 1) ? 'file' : 'files'}</span> |
🤖 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` at line 2537, In the file-count display, replace the use of
stats.total with the files array length for both the rendered count and
singular/plural selection. Update the expression in the relevant template span
so commits always show files.length files, with appropriate handling when files
is unavailable.
2 participants
Introduce GitLab proxy endpoints and a full commits UI.
Server: add GitLab handlers (commit count with X-Total or binary-search fallback, avatar-by-email, commits list, commit detail that merges info + diff). Register routes and update CSP to allow esm.sh for client-side modules.
Client/web: add commits view and commit detail UI, responsive styles for index/repo pages, Shiki-based async diff highlighting (fallback to multiple CDNs), gravatar/md5 helper, avatar prefetch via new API, and resolve relative image/link paths in rendered markdown. Also call loadCommitCount to populate sidebar. Minor icon and layout tweaks.
Summary by CodeRabbit