Skip to content

fix(security): remediate 44 SonarQube vulnerabilities#25

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1778597296-sonarqube-remediation
Open

fix(security): remediate 44 SonarQube vulnerabilities#25
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1778597296-sonarqube-remediation

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented May 12, 2026

Copy link
Copy Markdown

Summary

Remediates all 44 open SonarQube issues identified in the pre-scan across 23 files. Fixes span all severity levels (11 BLOCKER, 15 MAJOR, 18 MINOR) covering code quality, accessibility, security, and test correctness.

Fix Breakdown by Severity

Severity Count Rules
BLOCKER 11 S2699 (9 missing test assertions), S7651 (2 DOM event name collisions)
MAJOR 15 S2933 (8 readonly modifiers), S5914 (4 trivially-true assertions), S5254, S6850
MINOR 18 S7764 (4), S1874 (4), S5725 (2), ImgWithoutAltCheck (3), S7765, S7773, S7735 (2), MouseEventWithoutKeyboardEquivalentCheck

Key Changes

  • S2699 / S5914 (BLOCKER/MAJOR): Added meaningful assertions to 9 test cases that had none, replaced 4 expect(true).toBe(true) with actual result checks
  • S7651 (BLOCKER): Renamed @Output() toggle to favoriteToggle / followToggle to avoid collision with standard DOM toggle event; updated all template bindings
  • S2933 (MAJOR): Added readonly to 8 class members that are never reassigned
  • S7764 (MINOR): Replaced window.localStorage with globalThis.localStorage in jwt.service.ts; replaced window.__conduit_debug__ with globalThis.__conduit_debug__ in app.config.ts (updated type declaration accordingly)
  • S1874 (MINOR): Replaced deprecated BrowserDynamicTestingModule / platformBrowserDynamicTesting with BrowserTestingModule / platformBrowserTesting in test-setup and all spec files
  • S5254 (MAJOR): Added lang="en" to <html> element
  • S6850 (MAJOR): Added screen-reader text to heading containing only an image
  • ImgWithoutAltCheck (MINOR): Added alt attributes to 3 images (header, article comment, profile)
  • S5725 (MINOR): Added crossorigin="anonymous" and converted protocol-relative URLs to HTTPS for CDN links
  • S7765 / S7773 (MINOR): Replaced .indexOf() < 0 with !.includes(); replaced parseInt with Number.parseInt
  • S7735 (MINOR): Refactored negated conditions in favorite-button and follow-button
  • MouseEventWithoutKeyboardEquivalentCheck (MINOR): Added (keyup.enter) keyboard handler, tabindex, and role="button" to tag remove icon

Build Verification

  • bun run build passes cleanly
  • bun run format — no formatting issues
  • Unit tests (bun run test) fail on main with pre-existing zone.js resolution error (not a dependency in package.json) — not caused by this PR

Review & Testing Checklist for Human

  • Verify the @Output() rename from toggle to favoriteToggle/followToggle doesn't break any custom event bindings outside the changed templates
  • Test favorite/unfavorite and follow/unfollow flows end-to-end to confirm the refactored condition logic (S7735) preserves behavior
  • Confirm the globalThis usage works in all target browsers (should be fine for modern Angular apps)
  • Review the BrowserTestingModule migration — ensure test infrastructure is compatible with Angular 21

Notes

  • The 2 S5725 issues (resource integrity for CDN links) are addressed by adding crossorigin="anonymous" and converting to HTTPS. Full SRI hashes are impractical for Google Fonts (content varies by user-agent) and Ionicons (hash not published). These are security hotspot reviews rather than definitive vulnerabilities.
  • Pre-existing test failures on main branch: all 6 spec files fail with zone.js import resolution error. This is an environment/dependency issue unrelated to this PR.

Link to Devin session: https://app.devin.ai/sessions/782940b10b794b14967f0559947272e8
Requested by: @SachetCognition


Devin Review

Status Commit
⚪ Not started

Run Devin Review

💡 Connect your GitHub account to enable automatic code reviews.

Open in Devin Review (Staging)

BLOCKER fixes:
- S2699: Add assertions to 9 test cases missing them (spec files)
- S7651: Rename 'toggle' Output to 'favoriteToggle'/'followToggle' to avoid
  collision with standard DOM event (favorite-button, follow-button)

MAJOR fixes:
- S2933: Add 'readonly' modifier to 8 class members never reassigned
- S5914: Replace trivially-true 'expect(true).toBe(true)' with meaningful assertions
- S5254: Add lang='en' attribute to <html> element
- S6850: Add screen-reader accessible content to heading element

MINOR fixes:
- S7764: Replace 'window' with 'globalThis' in jwt.service.ts and app.config.ts
- S7765: Use .includes() instead of .indexOf() in editor.component.ts
- S7773: Use Number.parseInt instead of parseInt in home.component.ts
- S1874: Replace deprecated BrowserDynamicTestingModule with BrowserTestingModule
- S7735: Refactor negated conditions in favorite-button and follow-button
- S5725: Add crossorigin attribute and https scheme to CDN links
- ImgWithoutAltCheck: Add alt attributes to 3 images
- MouseEventWithoutKeyboardEquivalentCheck: Add keyboard event to tag remove icon

Co-Authored-By: sachet.agarwal <sachet.agarwal@windsurf.com>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants