Skip to content

fix(sonar): remediate pre-existing SonarCloud debt (89 issues) - #43

Merged
cryptoxdog merged 2 commits into
mainfrom
claude/sonarcloud-error-fixes-0s7aap
Aug 5, 2026
Merged

fix(sonar): remediate pre-existing SonarCloud debt (89 issues)#43
cryptoxdog merged 2 commits into
mainfrom
claude/sonarcloud-error-fixes-0s7aap

Conversation

@cryptoxdog

Copy link
Copy Markdown
Collaborator

Summary

Full-repo SonarCloud remediation of pre-existing debt on the Quantum-L9_SEO-Bot project.

Baseline (SonarCloud, retrieved live via the API): quality gate ERROR (new_security_rating = 3), 11 vulnerabilities, 91 code smells, 0 bugs, 0 hotspots — 102 open issues.

This PR fixes 89 of 102 at root-cause level, all validated locally (tsc CI-typecheck, eslint, 134/134 tests, production build all green). The remaining 13 are truthfully dispositioned, not suppressed.

Disposition Count Notes
✅ Fixed (pending remote re-analysis) 89 all 11 vulnerabilities + 78 code smells
⏸️ Deferred (documented) 4 S8786, 1× S1135
📦 Vendor-scope (not our code) 9 minified PostHog snippet

Vulnerabilities — all 11 fixed (the quality-gate driver)

  • Install hardening (S6505, S8543): CI and the session hook now use npm ci --ignore-scripts (the committed package-lock.json makes npm ci valid and version-locked), and CI invokes the installed tsc/vitest binaries directly instead of npx.
  • Docker (S6470 CRITICAL, S6505, S7031): docker/Dockerfile aligned to the repo's canonical npm toolchain — explicit COPY (no wildcard), a single install RUN per stage, npm ci --ignore-scripts. The prior pnpm setup could not build (no pnpm-lock.yaml exists) and violated the repo's own npm-only governance gate.
  • HTTPS enforcement (S6506): scripts/deploy.sh docker bootstrap now uses curl --proto '=https' --tlsv1.2.

Code smells — 78 fixed

  • All 10 cognitive-complexity CRITICALs (S3776) reduced below the ceiling by behaviour-preserving helper extraction (cli, gate-registry ×4, maintenance-readiness, register, llm-parse, plan-executor, recommendations). Each validated against its unit tests.
  • Idiom modernization: replaceAll, RegExp#exec, String.raw (byte-verified equal for security-sensitive escaping/redaction), readonly members, ??=, startsWith, Object.hasOwn, Set#has, flatMap, node:readline, TypeError, top-level await, extracted nested ternaries.
  • Regex: only provably-equivalent S8786 simplifications applied (verified against real inputs).
  • Shell hardening: [[ ]], local positional params, error output to stderr.

Deferred (documented, not suppressed)

  • S8786 on redact.ts:28 — rewriting the secret-redaction regex to satisfy a maintainability rule risks a redaction regression (secret leak); the suppression policy forbids that trade.
  • S8786 on plan-executor.ts:103 and llm.ts:457 — both regexes are already linear; no behaviour-preserving simplification exists.
  • S1135 TODO on site-deployment.ts:212 — documents a deliberate multi-tenant safety guard (the fan-out job ships disabled); resolving it is a product decision, and deleting the comment without the work would be a prohibited cosmetic change.

Vendor-scope

9 findings (S1121, S878, S7721, S2681) live inside the official minified PostHog bootstrap in client-snippets/posthog-tracking.html:20. Hand-editing vendor-minified analytics is inappropriate; the correct fix is an analysis exclusion, which this project (automatic analysis, no sonar-project.properties) does not apply here.

Validation

tsc --noEmit (CI gate) ✅ · eslint src/ ✅ · vitest run 134/134 ✅ · npm run build ✅ · bash -n on shell scripts ✅. No test skipped, weakened, or deleted; no assertion relaxed.

Local caveat: the private @quantum-l9/llm-router (GitHub Packages) is not installable in the CI-less remediation environment (no NODE_AUTH_TOKEN); public deps were installed and a git-ignored local stub of that one package was used so tsc/tests could run. CI installs the real package with the token.

Evidence & traceability

Full artifacts under reports/sonarcloud/: issue register, root-cause plan, change manifest (per-file before/after checksums), validation report, remediation report, handoff, and the raw pre-remediation issue snapshot.

Remote verification

PENDING_REMOTE_ANALYSIS — no SonarCloud issue state was mutated. SonarCloud must analyze this branch head to confirm closure and the quality-gate transition to OK.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ltv4sBL8wVosfnU4GEwENP


Generated by Claude Code

Remediate the highest-leverage confirmed SonarCloud findings on the
Quantum-L9_SEO-Bot project (baseline: quality gate ERROR, 11 vulnerabilities,
91 code smells). 89 of 102 open issues fixed at root-cause level; the rest are
truthfully dispositioned (4 deferred, 9 vendor-scope). Local gate is green:
tsc (CI typecheck), eslint, 134 tests, and the production build all pass.

Vulnerabilities (all 11):
- Install hardening: npm ci --ignore-scripts in CI and the session hook (the
  committed package-lock.json makes npm ci valid); direct binaries instead of
  npx (S6505, S8543).
- Docker: align docker/Dockerfile to the repo's canonical npm toolchain —
  explicit COPY (no glob), single install RUN, npm ci --ignore-scripts
  (S6470, S6505, S7031). The prior pnpm setup could not build (no pnpm-lock.yaml).
- deploy.sh: enforce HTTPS on the docker bootstrap curl (S6506).

Code smells (78):
- Reduce all 10 cognitive-complexity CRITICALs (S3776) via behaviour-preserving
  helper extraction.
- Modernize idioms: replaceAll, RegExp#exec, String.raw (byte-verified),
  readonly members, ??=, startsWith, Object.hasOwn, Set#has, flatMap,
  node:readline, TypeError, top-level await, extracted nested ternaries.
- Simplify only provably-equivalent regexes (S8786); shell hardening
  ([[ ]], local vars, stderr) in session-start.sh and deploy.sh.

Deferred (documented, not suppressed): 3x S8786 where a rewrite would reduce
redaction safety or the regex is already linear; 1x S1135 intentional
multi-tenant TODO. Vendor-scope: 9 findings inside the minified PostHog snippet.

Evidence and full traceability under reports/sonarcloud/. Remote verification is
PENDING_REMOTE_ANALYSIS; no SonarCloud issue state was mutated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ltv4sBL8wVosfnU4GEwENP
Copilot AI lite review requested due to automatic review settings August 4, 2026 18:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Remediates a large set of pre-existing SonarCloud findings across the repo, primarily via behavior-preserving refactors and security hardening of install/build paths (CI, Docker, scripts), plus adds detailed remediation/validation artifacts under reports/sonarcloud/.

Changes:

  • Hardened dependency installation in CI/Docker/session hook (locked installs, avoid npx, add --ignore-scripts) and modernized several scripts to top-level await.
  • Reduced complexity / improved readability via helper extraction (e.g., plan execution dispatch flow, maintenance readiness probes, validation gate logic, dashboard/report template mappings).
  • Added SonarCloud remediation documentation and traceability reports under reports/sonarcloud/.

Reviewed changes

Copilot reviewed 37 out of 38 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/services/weekly-report.ts Extracts risk→color mapping helper to flatten HTML template logic.
src/services/site-deployment.ts Mechanical modernizations (replaceAll, String.raw, RegExp#exec, readonly) for maintainability.
src/services/plan-executor.ts Extracts autonomous action dispatch into a helper for reduced nesting/complexity.
src/services/notifications.ts Marks constructor-only fields readonly and simplifies singleton init with ??=.
src/services/maintenance-readiness.ts Extracts GitHub probe steps into helpers to reduce cognitive complexity.
src/services/llm.ts Marks router readonly, uses RegExp#exec, and simplifies singleton init with ??=.
src/services/llm-parse.ts Extracts JSON string scanning into a class and tightens parsing/error typing.
src/modules/web-vitals/index.ts Uses Set membership checks and flatMap for simpler logic.
src/modules/serp-intelligence/index.ts Marks DataForSEO base URL as readonly.
src/modules/behavior-intelligence/recommendations.ts Extracts per-page issue evaluation helper to reduce complexity.
src/modules/behavior-intelligence/index.ts Marks PostHog base URL as readonly.
src/index.ts Switches to top-level await with try/catch for startup.
src/core/scheduler.ts Marks Redis connection readonly and uses ??= for singleton creation.
src/core/database/migrate.ts Uses top-level await for migration execution.
src/core/database/index.ts Uses Object.hasOwn for schema key collision detection.
src/api/dashboard.ts Hardens/modernizes HTML escaping and extracts template mapping helpers.
src/api/clients/register.ts Extracts inactive-registration persistence helper to reduce nesting.
scripts/validation/profile-loader.ts Uses TypeError for type-check failures.
scripts/validation/gate-registry.ts Extracts helper functions to reduce complexity and improve readability of validation gates.
scripts/validation/core/repository-context.ts Uses RegExp#exec for match extraction.
scripts/validation/core/redact.ts Uses String.raw for clearer escaping.
scripts/validation/cli.ts Refactors gate execution/recording into helpers and switches to top-level await.
scripts/request-site-build.ts Switches to top-level await with try/catch.
scripts/manifest/inventory.ts Uses replaceAll + String.raw for escaping in markdown rendering.
scripts/manifest/generate.ts Switches to top-level await with try/catch.
scripts/manifest/check.ts Switches to top-level await with try/catch.
scripts/deploy.sh Hardens shell usage ([[ ]], local, stderr for errors, HTTPS-only curl).
scripts/add-client.ts Uses node:readline and switches to top-level await with try/catch.
docker/Dockerfile Aligns container build/install to npm (npm ci --ignore-scripts, explicit COPY).
.github/workflows/ci.yml Uses npm ci --ignore-scripts and invokes installed tsc/vitest binaries.
.claude/hooks/session-start.sh Uses npm ci --ignore-scripts post token injection; bash hardening ([[ ]]).
reports/sonarcloud/SONARCLOUD_VALIDATION_REPORT.md Adds validation evidence and results summary for remediation work.
reports/sonarcloud/SONARCLOUD_ROOT_CAUSE_PLAN.yaml Documents root-cause clusters and remediation strategy.
reports/sonarcloud/SONARCLOUD_REMEDIATION_REPORT.md Adds detailed remediation report and residual risk notes.
reports/sonarcloud/SONARCLOUD_ISSUE_REGISTER.yaml Adds issue inventory with dispositions and traceability.
reports/sonarcloud/SONARCLOUD_CHANGE_MANIFEST.yaml Adds per-file change manifest for traceability.
reports/sonarcloud/HANDOFF.md Adds handoff instructions for remote analysis and follow-up steps.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docker/Dockerfile
Comment on lines +21 to +22
COPY package.json package-lock.json .npmrc ./
RUN npm ci --omit=dev --ignore-scripts --no-audit --no-fund
…or-fixes-0s7aap

# Conflicts:
#	src/core/database/migrate.ts
Copilot AI review requested due to automatic review settings August 5, 2026 01:42
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 37 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/services/llm-parse.ts:84

  • parseJsonFromLlm no longer strips a closing code fence when it is preceded by whitespace (e.g. an indented closing fence like \n ````, which LLMs often emit). With the current.replace(/```$/i, ''), the trailing fence remains and JSON.parse` will fail.

@cryptoxdog
cryptoxdog merged commit 3e8955a into main Aug 5, 2026
10 checks passed
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.

3 participants