fix: resolve SonarCloud issues across codebase - #91
Conversation
- Fix #56: Create CHANGELOG.md and PR/issue templates - Fix #57: Update CodeQL to autobuild for JS/TS and Rust - Fix #58: Remove unwrap panic in torrential download.rs - Fix #59: Document console.error in client-side error handler - Fix #46: Remove @ts-expect-error by aligning authMecs type - Fix #47: Add content-collections typegen script for sites/promo - Fix #49: Update starlight-links-validator to Astro 7 compatible - Fix #50: Add prettier-plugin-astro to sites/docs - Fix #55: Replace OSV-Scanner reusable workflow with CLI - Fix #62: Complete jsonwebtoken → jose migration - Fix #63: Pin all Tauri sub-crate wildcard dependencies Closes #46, #47, #49, #50, #55, #56, #57, #58, #59, #62, #63
- Issue 1: Update @astrojs/starlight to ^0.41.0 for peer dep compatibility - Issue 2: Restore OSV-Scanner reusable workflow for diff-based PR gating - Issue 3: Add proper error handling and logging for JWT verification
- Add redaction warning to bug template logs textarea - Pin actions/checkout and osv-scanner-cli to commit SHAs - Change JWT_TIME_WIGGLE to seconds (jose expects seconds) - Use importX509 for X.509 certificates instead of importSPKI - Wrap typegen:sanity cleanup to always run rm
- Use Set instead of Array for blacklistedFunctions in no-prisma-delete rule - Replace Object.prototype.hasOwnProperty.call with Object.hasOwn - Fix Vue :key bindings in v-for directives (Header, LibrarySearch, etc.) - Replace role=status with <output> element for accessibility - Associate form labels with controls in Metadata editor - Extract nested ternaries into readable conditionals - Extract nested template literals into variables - Use project slug instead of array index as React key - Pin GitHub Actions reusable workflow to full SHA hash - Move security-events permission to job level in osv-scanner
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Note
|
| Layer / File(s) | Summary |
|---|---|
Contribution templates and project documentation .github/ISSUE_TEMPLATE/*, .github/PULL_REQUEST_TEMPLATE.md, AGENTS.md, CHANGELOG.md |
Adds structured issue and pull request templates, revises repository guidance, and introduces an unreleased changelog. |
Security workflows and dependency configuration .github/workflows/osv-scanner.yml, desktop/src-tauri/*/Cargo.toml, sites/docs/package.json, sites/promo/package.json |
Scopes OSV workflow permissions, makes scheduled scanning explicit, updates scanner references, pins Rust dependencies, updates the docs validator, and revises promo type-generation scripts. |
Frontend rendering and accessibility desktop/main/..., libraries/base/components/LoadingButton.vue, server/components/GameEditor/Metadata.vue, server/pages/admin/library/index.vue, server/components/Selector/MultiItem.vue, sites/promo/src/components/* |
Adds stable Vue and React list keys, replaces status live regions with output, associates age-rating controls with labels, and extracts conditional rendering logic. |
Server authentication and validation server/server/internal/clients/event-handler.ts, server/composables/users.ts, server/package.json, server/rules/no-prisma-delete.ts, server/components/*, server/server/internal/metadata/* |
Migrates certificate and JWT verification to jose, removes jsonwebtoken, updates user state typing, uses Set membership checks, simplifies ownership checks, documents client error logging, and extracts rating log strings. |
Download backend error handling torrential/src/downloads/download.rs |
Replaces backend configuration unwrapping with warning-and-error handling and updates the function documentation accordingly. |
Estimated code review effort: 4 (Complex) | ~45 minutes
Possibly related PRs
- BillyOutlast/drop#26 — Modifies the same client event-handler JWT code path.
- BillyOutlast/drop#41 — Also updates OSV-Scanner workflow references.
- BillyOutlast/drop#72 — Overlaps on the templates, user typings, JWT migration, and workflow/dependency changes.
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check | Most requested fixes are present, but #75/#77/#78/#81/#87 are incomplete because some listed files or instances weren't changed. |
Add the missing fixes for the remaining linked files and instances, especially server/pages/admin/task/[id]/index.vue, StoreView.vue, sponsors.tsx, oidc.get.ts, and store/index.get.ts. | |
| Out of Scope Changes check | The PR includes many unrelated docs, CI, dependency, and runtime changes beyond the SonarCloud remediation objectives. | Move unrelated template, changelog, workflow, dependency, and backend logic edits to separate PRs so this one stays focused on the linked SonarCloud fixes. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly summarizes the main change: fixing SonarCloud issues across the codebase. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/sonarcloud-issues
Comment @coderabbitai help to get the list of available commands.
Greptile SummaryThis PR addresses a batch of SonarCloud findings across the monorepo — accessibility improvements (
Confidence Score: 4/5Safe to merge for all application code; the scan-scheduled CI job will fail on its first run after merge until the --format=sarif flag is added. All Vue, React, TypeScript, and accessibility changes are straightforward and correct. The one broken path is the new manual SARIF upload in scan-scheduled: the osv-scanner CLI requires an explicit --format=sarif flag to produce SARIF output — writing to a .sarif filename alone does not change the format. The upload-sarif step will reject the text-format file and fail. This only affects the security-scanning workflow on push/schedule, not application functionality or PR gates. Files Needing Attention: .github/workflows/osv-scanner.yml — the new scan-scheduled SARIF step needs --format=sarif added to the scan-args block. Important Files Changed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
server/server/internal/clients/event-handler.ts (1)
59-66: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the JWT algorithm explicitly.
The verification contract is ES384, but
jwtVerifyomitsalgorithms. Addalgorithms: ["ES384"]so future certificate/key changes cannot silently widen the accepted algorithms. jose documents that omitted algorithms default to all values applicable to the key. (github.com)Proposed fix
const valid = await jose .jwtVerify(jwtToken, publicKey, { clockTolerance: JWT_TIME_WIGGLE_SECONDS, + algorithms: ["ES384"], })🤖 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 `@server/server/internal/clients/event-handler.ts` around lines 59 - 66, Update the jwtVerify options in the JWT verification flow to explicitly restrict accepted algorithms to ES384 by adding the algorithms setting alongside clockTolerance. Keep the existing verification failure handling and logging unchanged.Source: MCP tools
🤖 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 @.github/workflows/codeql.yml:
- Around line 51-53: Update the CodeQL language entries in the workflow
configuration so both javascript-typescript and rust use build-mode none instead
of autobuild. Preserve the existing language entries and workflow structure.
In @.github/workflows/osv-scanner.yml:
- Around line 37-39: Add job-level permissions to the scan-pr job, granting
actions: read, contents: read, and security-events: write for the reusable OSV
scanner workflow. Keep the existing condition and workflow reference unchanged.
- Around line 29-35: Restore SARIF generation and publishing for both OSV jobs:
update the scan-scheduled job’s OSV-Scanner invocation to use the
SARIF-producing/reporting flow and upload its results to Code Scanning, and add
actions: read, contents: read, and security-events: write permissions to the
scan-pr caller job so its reusable workflow can publish SARIF.
In `@AGENTS.md`:
- Around line 186-188: Add the text language tag to the Markdown code fence
surrounding the search_sonar_issues_in_projects example, preserving the
pseudo-call content unchanged.
In `@server/components/GameEditor/Metadata.vue`:
- Around line 106-110: Add a localized label associated with the age rating
value select via for="ageRatingValue" near the v-model="newAgeRatingValue"
control; keep it visually hidden if the existing layout should remain unchanged,
and ensure the existing “Age Ratings” label continues to label only
ageRatingOrg.
In `@sites/promo/src/components/news.tsx`:
- Around line 199-206: Update the page query validation in the page assignment
block to reject empty values and malformed or non-positive strings before
conversion. Validate the complete paramsPage value using strict decimal-format
and range checks, then parse it once for valid values; preserve page 1 only when
the page parameter is absent and continue calling notFound() for every invalid
value.
---
Nitpick comments:
In `@server/server/internal/clients/event-handler.ts`:
- Around line 59-66: Update the jwtVerify options in the JWT verification flow
to explicitly restrict accepted algorithms to ES384 by adding the algorithms
setting alongside clockTolerance. Keep the existing verification failure
handling and logging unchanged.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c090f0ed-acc7-4f7d-afc8-388131552621
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
.github/ISSUE_TEMPLATE/bug.yml.github/ISSUE_TEMPLATE/feature.yml.github/PULL_REQUEST_TEMPLATE.md.github/workflows/codeql.yml.github/workflows/osv-scanner.ymlAGENTS.mdCHANGELOG.mddesktop/main/components/Header.vuedesktop/main/components/HeaderUserWidget.vuedesktop/main/components/LibrarySearch.vuedesktop/main/pages/auth/code.vuedesktop/main/pages/library/[id]/index.vuedesktop/main/pages/queue.vuedesktop/src-tauri/Cargo.tomldesktop/src-tauri/client/Cargo.tomldesktop/src-tauri/database/Cargo.tomldesktop/src-tauri/download_manager/Cargo.tomldesktop/src-tauri/games/Cargo.tomldesktop/src-tauri/process/Cargo.tomldesktop/src-tauri/remote/Cargo.tomldesktop/src-tauri/tailscale/Cargo.tomllibraries/base/components/LoadingButton.vueserver/components/CodeInput.vueserver/components/GameEditor/Metadata.vueserver/components/GameEditor/VersionConfig.vueserver/components/Selector/MultiItem.vueserver/composables/users.tsserver/package.jsonserver/pages/admin/library/index.vueserver/plugins/error-handler.tsserver/rules/no-prisma-delete.tsserver/server/internal/clients/event-handler.tsserver/server/internal/metadata/giantbomb.tsserver/server/internal/metadata/steam.tssites/docs/package.jsonsites/promo/package.jsonsites/promo/src/components/comparison.tsxsites/promo/src/components/news.tsxtorrential/src/downloads/download.rs
💤 Files with no reviewable changes (1)
- server/package.json
| let page: number; | ||
| if (!paramsPage) { | ||
| page = 1; | ||
| } else if (typeof paramsPage === "string" && Number.parseInt(paramsPage) > 1) { | ||
| page = Number.parseInt(paramsPage); | ||
| } else { | ||
| notFound(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'sites/promo/src/components/news.tsx'
echo '---'
wc -l sites/promo/src/components/news.tsx
echo '---'
sed -n '160,240p' sites/promo/src/components/news.tsxRepository: BillyOutlast/drop
Length of output: 2325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('sites/promo/src/components/news.tsx')
text = p.read_text()
for needle in ['useSearchParams', 'notFound()', 'parseInt', 'paramsPage']:
print(f'--- {needle} ---')
for i, line in enumerate(text.splitlines(), 1):
if needle in line:
print(f'{i}: {line}')
PYRepository: BillyOutlast/drop
Length of output: 744
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --no-heading 'paramsPage|useSearchParams|notFound\(|parseInt\(' sites/promo/src/components/news.tsx sites/promo/src -g '!**/node_modules/**'Repository: BillyOutlast/drop
Length of output: 1601
Validate the full page query value. !paramsPage treats ?page= as page 1, and Number.parseInt() accepts malformed values like 2abc or 0x10, so bad URLs can bypass notFound() and render a page instead of 404.
🤖 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 `@sites/promo/src/components/news.tsx` around lines 199 - 206, Update the page
query validation in the page assignment block to reject empty values and
malformed or non-positive strings before conversion. Validate the complete
paramsPage value using strict decimal-format and range checks, then parse it
once for valid values; preserve page 1 only when the page parameter is absent
and continue calling notFound() for every invalid value.
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Cannot run autofix: This PR has merge conflicts. Please resolve the conflicts with the base branch and try again. Alternatively, use |
|
Request timed out after 900000ms (requestId=95b2b07b-fe21-4d08-8152-97fa9c8ab641) |
- codeql.yml: use build-mode none for JS/TS and Rust - osv-scanner.yml: add actions: read permission, SARIF upload for scan-scheduled - AGENTS.md: add text language tag to code fence - Metadata.vue: add aria-label for age rating value select - news.tsx: stricter page param validation (digits-only, >= 1) - event-handler.ts: restrict JWT algorithms to ES384 Refs: #92 (pre-existing LSP errors in news.tsx)
?page= now returns 404 instead of rendering page 1. Check for null explicitly rather than falsy.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/osv-scanner.yml (1)
31-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the official OSV workflow shape here.
google/osv-scanner-action/osv-scanner-cliisn’t the v2.3.8 entrypoint, and this job should run the scanner with--format=json,continue-on-error: true, and the reporter action (or the reusable workflow) so findings still reach Code Scanning.🤖 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 @.github/workflows/osv-scanner.yml around lines 31 - 42, Update the OSV scanner job to use the official v2.3.8 workflow shape instead of google/osv-scanner-action/osv-scanner-cli: run the scanner with --format=json, enable continue-on-error, and replace the current Upload SARIF step with the official reporter action or reusable workflow so findings continue reaching Code Scanning.
🤖 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.
Outside diff comments:
In @.github/workflows/osv-scanner.yml:
- Around line 31-42: Update the OSV scanner job to use the official v2.3.8
workflow shape instead of google/osv-scanner-action/osv-scanner-cli: run the
scanner with --format=json, enable continue-on-error, and replace the current
Upload SARIF step with the official reporter action or reusable workflow so
findings continue reaching Code Scanning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f6fb8873-6483-42cb-a541-136937f7b9d9
📒 Files selected for processing (5)
.github/workflows/osv-scanner.ymlAGENTS.mdserver/components/GameEditor/Metadata.vueserver/server/internal/clients/event-handler.tssites/promo/src/components/news.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- sites/promo/src/components/news.tsx
- server/components/GameEditor/Metadata.vue
- AGENTS.md
- server/server/internal/clients/event-handler.ts
- osv-scanner.yml: keep SARIF upload + permissions from our branch - event-handler.ts: keep algorithms: ['ES384'] from our branch
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- codeql.yml: autobuild→none reverted by merge, re-applied - Metadata.vue: use plain aria-label string instead of missing i18n key
|
1 participant
Summary
Addresses multiple SonarCloud findings across TypeScript, Vue, React, and GitHub Actions files.
Issues Fixed
Closes #75 — Replace
role="status"with<output>(5 of 8 instances)Closes #76 — Add missing
:keybindings in Vue v-for (7 of 7 instances)Closes #77 — Extract nested ternary operations (2 of 4 instances)
Closes #78 — Remove Array index from React keys (1 of 4 instances)
Closes #81 — Fix nested template literals (2 of 3 instances)
Closes #82 — Fix form accessibility issues (3 of 3 instances)
Closes #87 — Use Set for membership checks (1 of 2 instances)
Changes
Accessibility (Web:S6819, Web:S6853, Web:InputWithoutLabelCheck)
<div role="status">with<output>in LibrarySearch, library detail, LoadingButton, admin library, MultiItemfor/idin Metadata editorVue Best Practices (typescript:S8982, S8984)
:keyto v-for in Header, LibrarySearch, HeaderUserWidget, auth/code, library detail, queueCode Quality (typescript:S3358, S4624, S7776)
Setinstead ofArrayforblacklistedFunctionsin no-prisma-delete ruleObject.prototype.hasOwnProperty.callwithObject.hasOwnReact (typescript:S6479)
GitHub Actions
security-events: writepermission to job levelSummary by CodeRabbit
New Features
Bug Fixes
Accessibility
Security