Follow-ups from toolchain adoption (#98): accessibility pass + triage pre-existing CodeQL/Sonar findings

[cevheri]

Context

During the toolchain adoption in #98 (Biome, oxlint, type-aware ESLint, knip, attw) three sets of findings
were deliberately deferred or surfaced as advisory. This issue consolidates them into one tracked follow-up.

Important: the 461-file one-shot Biome reformat in #98 makes pre-existing lines count as "new code", so the
CodeQL "new alerts" check and the SonarCloud quality gate re-attribute PRE-EXISTING findings to that PR (their
own caveat: "code changes were too large"). The flagged files' diffs were verified to be formatting-only.
After #98 merges to main, the new-code baseline resets, so these stop blocking unrelated PRs - but the
underlying findings remain in the codebase and are tracked here for real resolution. None were introduced by
#98.

1. Accessibility pass (oxlint jsx-a11y) - 60 findings

In #98 the 8 firing jsx-a11y rules were set to warn (not disabled) to avoid turning a tooling PR into an
accessibility refactor of vendored shadcn/ui plus many components needing markup/behaviour changes.

Rules currently at warn in .oxlintrc.json: no-static-element-interactions, click-events-have-key-events,
label-has-associated-control, prefer-tag-over-role, no-autofocus, control-has-associated-label,
no-noninteractive-element-interactions, anchor-has-content.

Representative locations: ResultsGrid.tsx, DataImportModal.tsx, MaskingSettings.tsx, QueryHistory.tsx,
StudioTabBar.tsx, SnapshotTimeline.tsx, SavedQueries.tsx, DataCharts.tsx, VisualExplain.tsx, plus
vendored ui/* (calendar, carousel, breadcrumb, pagination, input-group, field, item, spinner, button-group).

• Add keyboard handlers / roles for interactive non-button elements (click-events-have-key-events,
  no-static-element-interactions). Done in fix(a11y): resolve jsx-a11y warnings in app components (#100, a11y 1/2) #205: native buttons, stretched-link overlays, and proper
  tablist/tab semantics instead of hand-rolled keydown handlers.
• Associate labels with controls (label-has-associated-control, control-has-associated-label).
  Done in fix(a11y): resolve jsx-a11y warnings in app components (#100, a11y 1/2) #205 via htmlFor/id pairs, aria-labels for icon-only controls, and sr-only header text.
• Replace role="list"/etc. with semantic tags where it does not fight upstream shadcn (prefer-tag-over-role).
  Done in fix(a11y): resolve jsx-a11y warnings in app components (#100, a11y 1/2) #205: both role="button" sites became real buttons; vendored ui/* is excluded by a scoped
  override in chore(lint): promote jsx-a11y rules to error with scoped ui/* override (#100, a11y 2/2) #206 (fixing shadcn primitives in place would fork them from upstream).
• Review no-autofocus usages. Reviewed: all six are user-initiated focus management (inline editors,
  dialogs), so the rule is off by policy with an inline justification (chore(lint): promote jsx-a11y rules to error with scoped ui/* override (#100, a11y 2/2) #206).
• Once clean, promote the 8 jsx-a11y rules from warn to error in .oxlintrc.json.
  Done in chore(lint): promote jsx-a11y rules to error with scoped ui/* override (#100, a11y 2/2) #206: 7 rules promoted to error; no-autofocus is off per the review above (recorded deviation).

2. Triage CodeQL alerts (11: 5 high, 6 medium)

• js/sql-injection (high) x4 - src/lib/db/providers/sql/mssql.ts:222,
  src/lib/db/providers/document/mongodb.ts:277,281,287. Expected to be largely by-design (the product's
  purpose is executing user-supplied queries), but confirm each path is parameterised where it should be,
  then fix or dismiss-as-by-design with a written justification.
  Done: verified parameterisation (MSSQL binds via request.input; MongoDB uses the driver's structured
  API) and dismissed all four as by-design with written justifications.
• js/log-injection (medium) x6 - src/lib/logger.ts:85,96,99,102, src/lib/db/base-provider.ts:261,
  src/lib/db/factory.ts:58. logger.ts already sanitises control chars/newlines; verify the sanitiser
  covers each sink (likely false-positive) and dismiss, or route remaining sinks through it.
  Done: verified every sink sanitises inline; five dismissed as false positive, the stack-trace one as
  by-design (multi-line stacks, non-production only).
• js/tainted-format-string (high) x1 - src/lib/logger.ts:85. Confirm the format string is not
  user-controlled, or harden it.
  Done: real defect, fixed in fix(logger): use a constant console format string on the stack-trace path (#100) #204 (constant "%s\n%s" format string).
• js/path-injection (high) x2 - src/lib/db/providers/sql/sqlite.ts:176,177 (surfaced after this issue
  was written). Dismissed as by-design: the SQLite path is user-configured trusted server-side input
  (docs/providers/sqlite.md); NUL bytes rejected, path resolved.

3. SonarCloud new-code quality gate

new_coverage 92.9%, new_maintainability_rating 1, duplications 1.3%, hotspots 100% are all OK. Two
conditions fail, both pre-existing:

• new_security_rating = 3 - 25x Make sure using this pseudorandom number generator is safe
  (Math.random in src/components/TestDataGenerator.tsx). This is fake/sample-data generation, not
  security-sensitive; mark the hotspots Safe in SonarCloud (or add a documented justification).
  Done in fix(sonar): repair quality gate baseline and clear new-code issues #103: file-scoped, documented S2245 suppression.
• new_reliability_rating = 4 - remaining bugs (the route.ts constant-|| one was already fixed in feat(toolchain): adopt Biome, oxlint, attw; keep eslint-config-next + knip #98) -
  all fixed in fix(sonar): repair quality gate baseline and clear new-code issues #103 (the frozen sonar.projectVersion baseline was also fixed there: CI now injects it
  from package.json):
  • src/components/PivotTable.tsx:87 - provide a locale-aware compare function to .sort().
  • src/components/SchemaDiagram.tsx:220 - conditional returns the same value on both branches; simplify or fix.
  • src/components/DataCharts.tsx:777 - non-interactive element with a click handler (same class as the a11y item above).

Acceptance

• jsx-a11y findings resolved and the 8 rules promoted to error.
• Every CodeQL alert either fixed or dismissed with a written by-design justification.
• SonarCloud quality gate green on a follow-up PR (Math.random hotspots reviewed; reliability bugs fixed).

Refs: #98

[cevheri]

Status update after re-evaluating this issue against the current codebase (0.9.56) and the live alert lists.

Section 3 (SonarCloud new-code quality gate): done, via PR #103

• PivotTable.tsx now sorts with a localeCompare comparator.
• The tautological ternary in SchemaDiagram.tsx was removed (the component was later rebuilt in fix: rebuild ER diagram for large schemas and working PNG/SVG export #180 anyway).
• The DataCharts click handler received keyboard activation as part of the S1082 wave (activateOnKey helper).
• The Math.random hotspots in TestDataGenerator are suppressed with a file-scoped, documented S2245 exclusion (fake sample data, not crypto material) - the "documented justification" option from this issue's checklist.
• The root cause of the gate noise (frozen sonar.projectVersion baseline) is fixed permanently: CI injects the version from package.json, so the new-code baseline resets at every release.

Section 2 (CodeQL triage): not started, and the list has grown

All 11 alerts from the issue body are still open; line numbers have shifted (mssql.ts:222 is now :486, mongodb.ts:277/281/287 are now :307/311/317). Two additional high alerts are open that are not in the issue body:

• js/path-injection x2 - src/lib/db/providers/sql/sqlite.ts:176-177 (fs.existsSync/fs.mkdirSync on a directory derived from the user-supplied SQLite database path). Same "the product's purpose is connecting to user-specified databases" class as the sql-injection findings; needs the same fix-or-dismiss-with-written-justification treatment.

Current open total: 13 alerts (7 high, 6 medium).

Section 1 (jsx-a11y pass): still open, and the debt is drifting because the rules are warn

Current inventory: 58 warnings (issue said 60), but the content has shifted - some were fixed in #103 while code merged since added new ones (VisualExplain.tsx x3 from the explain series, plus ResultCard.tsx, SchemaDiff.tsx, login-form.tsx, which were not in the original list). warn does not fail CI, so new violations keep landing; promoting the 8 rules to error gets more expensive the longer this waits.

By rule: prefer-tag-over-role 12, label-has-associated-control 12, no-static-element-interactions 11, click-events-have-key-events 11, no-autofocus 6, control-has-associated-label 4, no-noninteractive-element-interactions 1, anchor-has-content 1.

Heaviest files: ResultsGrid.tsx (11), MaskingSettings.tsx (8), DataImportModal.tsx (6), vendored ui/* (~13 across 9 files).

Next steps

1. CodeQL triage: verify the logger sanitiser actually covers the six log-injection sinks, then fix or dismiss every alert (including the two path-injection ones) with a written justification.
2. a11y series in 2-3 PRs: app components first, then a policy decision for vendored ui/* (fix vs scoped override), finishing with the promotion of the 8 rules to error.

[cevheri]

All three sections are complete; closing.

Section 1 - accessibility pass (#205 + #206)

• fix(a11y): resolve jsx-a11y warnings in app components (#100, a11y 1/2) #205 cleared all 39 app-code jsx-a11y findings with TDD (16+ new a11y-semantics tests): native buttons where possible, stretched-link overlays for cards with nested actions, real tablist/tab semantics with roving tabindex, label associations, accessible names for icon-only controls, and honest exemptions (role="presentation" wrappers, aria-hidden resize handle, two documented oxlint false positives on dynamic td content). Three Copilot review rounds were applied in full - including two hit-target regressions and a dot-click regression caught during review.
• chore(lint): promote jsx-a11y rules to error with scoped ui/* override (#100, a11y 2/2) #206 locked the gate: 7 rules promoted to error (new violations now fail CI), no-autofocus off by policy (all usages are user-initiated focus management), vendored ui/* excluded via a scoped override. Both policy decisions carry inline justification comments in .oxlintrc.json.
• Recorded deviation from the acceptance criteria: "promote the 8 rules" became "promote 7, disable no-autofocus with justification".

Section 2 - CodeQL triage (#204 + dismissals)

12 alerts dismissed with written justifications (5 false positives where inline sanitizers are invisible to CodeQL; 7 by-design: a database IDE executes user-authored queries, the SQLite path is documented trusted server-side input, stack traces are intentionally multi-line outside production). One real defect fixed in #204 (tainted format string in the logger). Open code-scanning alerts: 0.

Section 3 - SonarCloud quality gate (#103)

All reliability bugs fixed, Math.random hotspots suppressed with a file-scoped documented justification, and the frozen new-code baseline root-caused and fixed (CI injects sonar.projectVersion from package.json).