Skip to content

[chore] fix sonar issues: replaceAll, globalThis, a11y, Readonly props, ReDoS - #5

Merged
YJack0000 merged 1 commit into
mainfrom
chore/sonar-sweep
Aug 23, 2026
Merged

YJack0000 merged 1 commit into
mainfrom
chore/sonar-sweep

Conversation

@YJack0000

Copy link
Copy Markdown
Contributor

Sweeps the 70 SonarQube issues open on pathorsAI_pensieve plus the 6 security hotspots. No behaviour change intended.

Fixed (70/70)

Rule What Where
S7781 replace(/…/g, …) → replaceAll (only where equivalent: string first arg or g-flag regex) lib/mcp.ts, lib/search.ts, lib/github.ts, lib/extract.ts, lib/markdown.ts, lib/auth.ts, app/o/[slug]/d/[...path]/route.ts
S7764 window → globalThis (SSR guard kept as typeof globalThis.window === "undefined") app/consent/consent-form.tsx, app/login/login-panel.tsx
S7758 charCodeAt → codePointAt, String.fromCharCode → fromCodePoint (all byte values 0–255, so identical) lib/github.ts, route.ts
S6594 String.match → RegExp.exec lib/markdown.ts, lib/extract.ts, graph-view.tsx
S6759 props typed Readonly<…> 10 components
S6479 array-index key → stable key (m.email; skeleton widths are distinct) members/page.tsx, graph-view.tsx
S6478 Folder / DocLink / RelLink lifted out of GraphView into module scope, dependencies passed as props graph-view.tsx
S6848 / S6844 / S1082 clickable <span>s (disclosure arrow, folder name) and a bare <a> ("清除") become <button type="button"> with aria-expanded / aria-label graph-view.tsx
S3358 nested ternaries flattened (edge alpha, node fill, label truncation, accept-client branch) graph-view.tsx, accept-client.tsx
S7735 !session ? … : … inverted accept-client.tsx
S2681 two independent ifs on one line get braces and their own lines graph-view.tsx
S7769 Math.sqrt(dx*dx + dy*dy) → Math.hypot(dx, dy) graph-view.tsx
S1090 <iframe> gets a title graph-view.tsx
S4325 dropped the non-null assertion in favour of an explicit DATABASE_URL check lib/db.ts

Notes on the two judgement calls:

  • S2681 in LocalGraph was not a real bug. if (e.from === center) near.add(e.to); if (e.to === center) near.add(e.from); — an edge can touch center at either end and both checks were already independent and correct. Only the one-line layout was misleading; it now has braces and a comment.
  • S4325 in lib/db.ts looked like a false positive — tsc confirms process.env.DATABASE_URL! genuinely needs the assertion under strict. Rather than suppress it, the variable is now checked explicitly. neon() already threw at module load when the variable was unset, so the failure point is unchanged; the message is just clearer.

Security hotspots

# Location Verdict
1 lib/extract.ts:44 S5852 SAFE — /<[^>]+>/g: the negated class and the closing > are disjoint, so the match is deterministic and runs in linear time with no backtracking.
2 lib/github.ts:9 S5852 FIXED — /=+$/ (polynomial on a run of =) replaced with a while (s.endsWith("=")) loop; equivalent for base64 output and unambiguously linear.
3 lib/markdown.ts:13 S5852 FIXED — dropped the trailing $ from /^(\w[\w-]*):\s*(.*)$/, removing the \s*/.* backtracking; the greedy .* already runs to the end of the (newline-free) frontmatter line.
4 lib/markdown.ts:17 S5852 FIXED — /^#\s+(.+)$/m → /^#\s+(\S.*)$/m; \S is disjoint from the preceding \s+, so each backtrack fails in O(1).
5 app/o/[slug]/graph-view.tsx:44 S2245 SAFE — Math.random() only jitters initial node positions in the force-directed layout; it is decorative, never used for tokens, ids, or any security decision.
6 app/o/[slug]/graph-view.tsx:44 S2245 SAFE — same line, the y-axis counterpart of the same layout jitter; not a security context.

Both markdown rewrites and the base64 trim were diffed against the originals over a table of inputs (frontmatter lines, headings with mixed whitespace, padded/unpadded base64) — identical output in every case.

Verification

bunx tsc --noEmit and bun run build (Next.js 15) both clean. Note this repo has no CI workflow yet.

…s, ReDoS

Sweeps the 70 SonarQube issues open on pathorsAI_pensieve plus the 6
security hotspots. No behaviour change intended.

- S7781 replace(/…/g) -> replaceAll (mcp, search, github, extract,
  markdown, auth, route)
- S7764 window -> globalThis (consent-form, login-panel)
- S7758 charCodeAt/fromCharCode -> codePointAt/fromCodePoint
- S6594 String.match -> RegExp.exec (markdown, extract, graph-view)
- S6759 props typed Readonly<…> (10 components)
- S6479 array-index keys -> stable keys (members table, skeleton rows)
- S6478 Folder/DocLink/RelLink lifted out of GraphView to module scope,
  deps passed as props
- S6848/S6844/S1082 clickable spans and a bare <a> become <button>s with
  aria-expanded/aria-label
- S3358 nested ternaries flattened (graph-view draw loop, accept-client)
- S7735 negated condition inverted (accept-client)
- S2681 the two independent `if`s in LocalGraph get braces and their own
  lines — both branches were already correct, only the layout was
  misleading
- S7769 Math.sqrt(dx*dx+dy*dy) -> Math.hypot
- S1090 iframe gets a title
- S4325 drop the non-null assertion in lib/db.ts in favour of an explicit
  DATABASE_URL check (neon() already threw at module load without one)

Hotspots: two S5852 rewritten to non-backtracking equivalents
(github.ts b64url trailing "=", markdown.ts frontmatter/h1 patterns);
the rest reviewed and left as-is.

Verified: bunx tsc --noEmit and next build both clean.
@YJack0000
YJack0000 merged commit 27a0877 into main Aug 23, 2026
1 check failed
@github-actions

Copy link
Copy Markdown

❌ SonarQube Quality Gate ERROR — pathorsAI_pensieve

failed condition value threshold
new_security_hotspots_reviewed 0.0 ≥ 100
new_violations 1 ≤ 0

1 open issue on this PR:

  • MINOR typescript:S7741 — Compare with undefined directly instead of using typeof. (app/consent/consent-form.tsx:7)

YJack0000 added a commit that referenced this pull request Aug 23, 2026
…ectly (#6)

Follow-up to #5. `globalThis.window` is a property access that never throws,
so the `typeof` guard it inherited from the bare `window` identifier is no
longer needed.
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.

1 participant