Skip to content

fix(audit): SonarCloud findings — real fixes, false-positive triage, a11y labels - #305

Merged
Rem7474 merged 6 commits into
devfrom
fix/sonarqube-audit-findings
Aug 28, 2026
Merged

Rem7474 merged 6 commits into
devfrom
fix/sonarqube-audit-findings

Conversation

@Rem7474

@Rem7474 Rem7474 commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • Revue manuelle des 8 signaux BLOCKER/CRITICAL bug/vulnerability remontés par SonarCloud (voir AUDIT-SONARQUBE-2026.md pour le détail de la vérification).
  • 2 vrais correctifs : vérification hostname/chaîne de confiance manquante dans la sonde SSL (ssl.go, ne remontait que l'expiration), vérification d'origine absente sur le listener message du service worker.
  • 1 clarification bénigne : tri explicite localeCompare au lieu du tri par défaut (déjà correct fonctionnellement).
  • 3 faux positifs documentés (NOSONAR + justification) : hash bcrypt factice anti-timing-attack, empreinte MD5 non-cryptographique, migration SQL déjà appliquée (non modifiée, par convention du projet).
  • Ajout de AUDIT-SONARQUBE-2026.md : chiffres bruts, triage des BLOCKER/CRITICAL, analyse des deux patterns à fort volume (go:S2077 échantillonné sur tous les fichiers distincts — aucune injection réelle trouvée ; Web:InputWithoutLabelCheck/S6853, 450 issues d'accessibilité), vérification des 3 alertes Dependabot ouvertes (toutes déjà obsolètes — versions résolues déjà patchées), et plan de suite priorisé.

Test plan

  • go build ./... (server)
  • go vet ./internal/synthetic/... ./internal/database/...
  • npm run typecheck (frontend)
  • Revalidation SonarCloud sur push de cette branche

🤖 Generated with Claude Code

root added 2 commits August 28, 2026 09:25
…alse positives

- ssl.go: independently verify hostname/chain of trust after the
  intentionally-InsecureSkipVerify handshake, so a wrong hostname or
  untrusted CA is surfaced in last_error instead of silently passing
  (go:S4830/go:S5527 were true gaps, not just noisy — expiry alone was
  the only thing ever reported).
- service-worker.js: reject 'message' events whose origin isn't our own
  before acting on SKIP_WAITING/CLEAR_CACHE (javascript:S2819).
- useGlobalScheduledTasks.ts: explicit locale-aware compare in the host
  name sort instead of relying on default lexicographic sort() (typescript:S2871).
- db_hosts.go / db_web_logs.go: NOSONAR + justification on the two
  BLOCKER/CRITICAL secret-scanner false positives (a non-secret dummy
  bcrypt hash used only for timing-attack normalisation, and a
  non-cryptographic MD5 dedup fingerprint) so the SonarCloud dashboard
  reflects reality instead of carrying two permanently-red blockers.
…n plan

Documents the manual verification behind fix/sonarqube-audit-findings'
code changes (which signals were real vs. systematic false positives)
and lays out a prioritized plan for the remaining backlog (a11y labels,
S2077 exclusion, cognitive complexity) instead of a raw issue dump.
@github-actions github-actions Bot added documentation Improvements or additions to documentation server frontend database labels Aug 28, 2026
First pass on the 450 Web:InputWithoutLabelCheck/S6853 SonarCloud
findings (AUDIT-SONARQUBE-2026.md §2.2): the 8 components concentrating
the most occurrences, covering ~half the total count.

Two patterns fixed throughout:
- A <label> with real text next to its <input>/<select> gets a
  matching id/for pair (same id reused across mutually exclusive
  v-if/v-else alternates of one field, since only one is ever mounted).
- A <label> used as a section heading with no associated control
  (radio-card groups, checkbox groups, button groups) becomes a <div>
  with the same classes — it was never really a form label.
- Free-standing table-row inputs (no visible label, column header
  provides context) get an aria-label instead.
@Rem7474 Rem7474 changed the title fix(server,frontend): SonarCloud audit — real findings + false-positive triage fix(audit): SonarCloud findings — real fixes, false-positive triage, a11y labels Aug 28, 2026
root added 3 commits August 28, 2026 10:31
- Fix a genuine golangci-lint failure (staticcheck QF1001): rewrite the
  negated-AND condition in ssl.go with De Morgan's law.
- Extract the last_error decision logic into a pure certificateIssues()
  helper and add tests (table-driven unit tests + two self-signed-cert
  integration tests + a dial-failure test) — SonarCloud's new-code gate
  requires 80% coverage on added lines, and this package had none.
- Fix 3 duplicate `id` attributes SonarCloud's HTML analyzer flagged as
  new bugs (Web:S7930): AlertRuleStepConditions.vue reused one id across
  two non-adjacent v-else-if branches, WebhookTrackerFields.vue reused
  three ids between the top-level git-tracker fields and the unrelated
  "linked repo" fields nested inside the docker branch. Both looked safe
  by Vue's runtime exclusivity but aren't structurally identical to the
  adjacent select/input v-if/v-else pattern the analyzer does accept, so
  every id is now unique regardless of runtime exclusivity.
- Fix the PR title to match the enforced Conventional Commits format
  (comma in a multi-word scope isn't a valid scope character).
…ponents with no coverage

MonitoringOverviewPanel, NetworkTopologyConfig, WebhookModal,
WebhookTrackerFields, AlertRuleTemplateModal and SettingsThreatDetectionCard
had zero dedicated tests before this — the id/for/aria-label attributes
added for the SonarCloud a11y pass were mechanically never exercised,
which is why SonarCloud's new-code coverage gate flagged this PR (11.5%
against an 80% requirement) despite the changes themselves being inert
attribute additions. These tests mount each component through its real
branches (probe/cert edit and create-shared forms, git/docker tracker
modes, the linked-repo sub-section, compose-mode hooks, per-host
discovered-port rows) and assert every new id/label pair and aria-label
actually renders, raising whole-file coverage on these 8 files from
near-zero to ~62% aggregate.
- sonar-project.properties: mark *_test.go as test code (sonar.tests +
  sonar.test.inclusions), matching what's already done for frontend
  *.spec.ts. Without it, a _test.go file's own new lines counted as
  "new code to cover" and scored 0% (go's coverage.out never
  instruments test files themselves) — ssl_test.go alone contributed
  52 of the PR's 79 uncovered new lines for exactly this reason.
- ssl_test.go: add a chain-with-intermediate test so the
  `intermediates.AddCert` loop in checkCertificate (only reachable
  when the peer presents more than one certificate) is actually hit —
  the existing self-signed-leaf tests never had a second cert to add.
- AlertRuleStepConditions.spec.ts / AlertRuleStepSource.spec.ts (new):
  neither child component had a dedicated test — the only existing
  coverage came indirectly through AlertRuleModal.spec.ts's cpu/memory
  characterization, which never touches the docker/proxmox/bandwidth/
  heartbeat metric branches these a11y fixes landed in. Cover each
  branch directly instead of a full metric matrix.
@sonarqubecloud

Copy link
Copy Markdown

@Rem7474
Rem7474 merged commit 3bea495 into dev Aug 28, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

database documentation Improvements or additions to documentation frontend server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant