fix: Prevent XSS vulnerability in report rendering (#546)#580
Open
SinghCharanjeet11 wants to merge 6 commits intofossasia:mainfrom
Open
fix: Prevent XSS vulnerability in report rendering (#546)#580SinghCharanjeet11 wants to merge 6 commits intofossasia:mainfrom
SinghCharanjeet11 wants to merge 6 commits intofossasia:mainfrom
Conversation
- Update biome schema version from 2.3.13 to 2.4.10 to match CLI - Add GitLab and Gitea support to roadmap in README - Highlight new multi-platform features in Features section - Document upcoming SCM analytics and Gitea integration Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Add commits fetching from GitLab projects with date filtering - Map commits to standard format for report generation - Include commits data in GitLab data processing - Add project metadata to commits for better tracking - Support author filtering using author_name parameter This enables complete feature parity with GitHub support: Projects fetching Merge requests fetchingIssues fetching Commits fetching
…upport feat: complete GitLab support with commits fetching
- Add commits fetching from GitLab projects with date filtering - Map commits to standard format for report generation - Include commits data in GitLab data processing - Add project metadata to commits for better tracking - Support author filtering using author_name parameter This enables complete feature parity with GitHub support: Projects fetching Merge requests fetchingIssues fetching Commits fetching
…upport feat: complete GitLab support with commits fetching
Apply escapeHtml() to all user-controlled data from GitHub/GitLab API responses before HTML insertion. Fixes XSS vulnerabilities in PR titles, issue titles, commit messages, and repository names. - Escape title, project, html_url in all PR/issue rendering - Escape commit.messageHeadline in commit lists - Escape reviewed PR titles and URLs - Preserve all existing HTML structure and functionality Security Impact: - Prevents arbitrary JavaScript execution from malicious API data - Protects GitHub/GitLab tokens and user activity data - Secures both extension popup and email compose window contexts Fixes fossasia#546
github-actions
Bot
added
javascript
Contributor
Reviewer's GuideApplies HTML escaping to all rendered GitHub/GitLab PR, MR, issue, and commit fields to fix an XSS vulnerability, and extends GitLab integration to fetch and render user commits alongside existing activity. Sequence diagram for GitLab activity and commit fetching into scrum reportsequenceDiagram
actor User
participant Extension
participant GitLabHelper
participant GitLabAPI
participant ReportRenderer
User->>Extension: Trigger scrum report generation
Extension->>GitLabHelper: Request GitLab activity data
GitLabHelper->>GitLabAPI: GET users, projects
GitLabAPI-->>GitLabHelper: users, projects
loop For each project
GitLabHelper->>GitLabAPI: GET merge requests and issues
GitLabAPI-->>GitLabHelper: merge requests, issues
GitLabHelper->>GitLabAPI: GET commits with author_name and date range
GitLabAPI-->>GitLabHelper: commits with project metadata
end
GitLabHelper-->>Extension: gitlabData with mergeRequests, issues, commits
Extension->>Extension: Map commits to standard format gitlabCommits
Extension->>ReportRenderer: writeGithubIssuesPrs for issues, merge requests, commits
ReportRenderer-->>User: HTML scrum report including GitLab commits
Updated class diagram for GitLab data and mapped commit itemsclassDiagram
class GitlabDataRaw {
user
projects
mergeRequests
issues
commits
comments
}
class GitlabDataProcessed {
mergeRequests
issues
commits
comments
user
}
class GitlabCommitItem {
string id
string short_id
string title
string message
string author_name
string author_email
string created_at
string project_id
string project_name
string project_url
string web_url
}
class MappedCommitItem {
string id
string sha
string message
string html_url
string project
string author_name
string author_email
}
GitlabDataRaw "1" --> "*" GitlabCommitItem : contains
GitlabDataProcessed "1" --> "*" GitlabCommitItem : includes
GitlabCommitItem --> MappedCommitItem : mapped_to
Flow diagram for escaping HTML in rendered report fieldsflowchart TD
APIResponse["GitHub and GitLab API data
titles, URLs, messages, project names"]
Renderer["Scrum report renderer
writeGithubIssuesPrs and related logic"]
Escape["escapeHtml applied to
title, project, html_url,
commit.messageHeadline"]
SafeHTML["Safe HTML list items
and links in report"]
Email["Email compose window /
extension popup"]
APIResponse --> Renderer
Renderer --> Escape
Escape --> SafeHTML
SafeHTML --> Email
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
githubCommitsDatavariable is only used for GitLab commits; consider renaming it (and related mapping keys) to a platform-agnostic name likegitlabCommitsDataorallCommitsDatato avoid confusion. - When building anchor tags,
html_urlis passed throughescapeHtmlbut still used as an href; you may want to additionally validate or normalize these URLs (e.g., ensuring http/https schemes) to reduce the risk of unsafe or broken links being rendered. - The GitLab commits fetch loop makes sequential requests with a fixed
setTimeoutdelay per project; for users with many projects this could be slow—consider batching or limited parallelism with a simple concurrency/rate-limit control instead of strictly serializing all calls.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `githubCommitsData` variable is only used for GitLab commits; consider renaming it (and related mapping keys) to a platform-agnostic name like `gitlabCommitsData` or `allCommitsData` to avoid confusion.
- When building anchor tags, `html_url` is passed through `escapeHtml` but still used as an href; you may want to additionally validate or normalize these URLs (e.g., ensuring http/https schemes) to reduce the risk of unsafe or broken links being rendered.
- The GitLab commits fetch loop makes sequential requests with a fixed `setTimeout` delay per project; for users with many projects this could be slow—consider batching or limited parallelism with a simple concurrency/rate-limit control instead of strictly serializing all calls.
## Individual Comments
### Comment 1
<location path="src/scripts/gitlabHelper.js" line_range="174-183" />
<code_context>
+ // Fetch commits from each project
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Per-project commit fetching is sequential and limited to 100 items, which may be slow and can miss commits for active projects.
The per-project loop fetches only a single page (`per_page=100`) with an added 100ms delay. This can (a) silently miss commits for very active projects due to missing pagination, and (b) be slow for users with many projects. Please either add pagination support (e.g., follow `X-Next-Page` or equivalent) or explicitly enforce and document the 100-commit limit in code. You could also improve throughput by batching, reducing the delay, limiting the queried projects, or running a bounded number of requests in parallel instead of fully sequentially.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+174
to
+183
| // Fetch commits from each project | ||
| let allCommits = []; | ||
| for (const project of allProjects) { | ||
| try { | ||
| const projectCommitsUrl = `${this.baseUrl}/projects/${project.id}/repository/commits?author_name=${username}&since=${startDate}T00:00:00Z&until=${endDate}T23:59:59Z&per_page=100&order_by=committed_date&sort=desc`; | ||
| const projectCommitsRes = await fetch(projectCommitsUrl, { headers }); | ||
| if (projectCommitsRes.ok) { | ||
| const projectCommits = await projectCommitsRes.json(); | ||
| allCommits = allCommits.concat( | ||
| projectCommits.map((commit) => ({ |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Per-project commit fetching is sequential and limited to 100 items, which may be slow and can miss commits for active projects.
The per-project loop fetches only a single page (per_page=100) with an added 100ms delay. This can (a) silently miss commits for very active projects due to missing pagination, and (b) be slow for users with many projects. Please either add pagination support (e.g., follow X-Next-Page or equivalent) or explicitly enforce and document the 100-commit limit in code. You could also improve throughput by batching, reducing the delay, limiting the queried projects, or running a bounded number of requests in parallel instead of fully sequentially.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a critical XSS (Cross-Site Scripting) security vulnerability in the Scrum Helper extension where malicious HTML/JavaScript in GitHub/GitLab API responses could execute in the extension context or email compose window.
Fixes #546
Problem
The extension was rendering user-controlled data from GitHub/GitLab API responses (PR titles, issue titles, commit messages, repository names) directly into HTML without sanitization. An
escapeHtml()function existed but was never used in report rendering, allowing potential XSS attacks.Attack Vectors:
<script>tags<img src=x onerror=alert('XSS')>' onclick='alert(1))Solution
Applied the existing
escapeHtml()function to all vulnerable locations insrc/scripts/scrumHelper.js:Changes Made:
PR/MR Rendering (Draft, Open, Closed, Merged states)
title,project,html_urlin all PR rendering pathscommit.messageHeadlinein commit listsIssue Rendering (Open, Closed states)
title,project,html_urlin all issue rendering pathsReviewed PRs Rendering
pr_arr.title,pr_arr.html_url(single PR format)pr_arr1.title,pr_arr1.html_url(list format)Security Impact
✅ Prevents arbitrary JavaScript execution from malicious API data
✅ Protects GitHub/GitLab tokens and user activity data
✅ Secures both extension popup and email compose window contexts
✅ Handles all HTML special characters:
&,<,>,",'Preservation
All existing functionality is preserved:
Testing
Type of Change
Checklist
npm run build)Summary by Sourcery
Fix XSS vulnerabilities in scrum report rendering and enhance GitLab integration for commits and documentation.
Bug Fixes:
Enhancements:
Documentation: