Skip to content

fix: resolve react-doctor security, bug & perf findings - #3801

Draft
ansmonjol wants to merge 2 commits into
mainfrom
fix/react-doctor-findings
Draft

ansmonjol wants to merge 2 commits into
mainfrom
fix/react-doctor-findings

Conversation

@ansmonjol

Copy link
Copy Markdown
Contributor

Fixes a batch of the highest-value react-doctor findings. Each change was verified against the tool, types pass, and affected tests are green.

Security

  • CI secret boundary (cypress.yml): install deps before any secret is exposed to a step env, with --ignore-scripts, so a malicious package script can't run arbitrary code or read secrets during install.
  • Mention XSS (Mention.schema.ts): escape id/label before writing them to innerHTML, closing a stored-XSS vector via mention labels.

Bugs (error severity)

  • Effect cleanup (ChargeFilter.tsx): clearTimeout in the effect cleanup so a stale timer can't fire on an unmounted node.
  • key before spread (RadioGroupField, RadioGroupFieldForTanstack, MultipleComboBox): a {...spread} could clobber the explicit key and scramble React's list tracking.

Performance

  • .find() in loops -> Map lookups (Invoices, useAddSubscription, useCreateCreditNote)
  • .includes() in loops -> Set (useFilters, ChargeFilter)
  • .map().filter(Boolean) double-pass -> flatMap (useFieldError, TextInputFieldForTanstack, PricingBlockView)
  • .sort()[0] -> single-pass reduce (getMostRecentPaymentMethodId)
  • length check before .every() (mapFromApiToForm)
  • JSON.parse(JSON.stringify()) -> structuredClone (CustomerInvoiceRegenerate)
  • hoist repeated property reads in loops (check-translations, TableCommands)

Config

  • doctor.config.json: ignore two verified false-positive unused-file hits (check-translations.js is a manually-run team CLI; CustomerInvoiceVoid.tsx is reachable via a lazy template-literal route import).

ansmonjol added 2 commits July 1, 2026 17:15
- cypress CI: install deps before secrets + --ignore-scripts
- Mention schema: escape id/label before innerHTML (XSS)
- ChargeFilter: clearTimeout cleanup in effect
- Radio + MultipleComboBox: key before spread
- check-translations / TableCommands: hoist repeated property reads
- doctor.config: ignore two verified false-positive unused-file hits
- index-map lookups replace find() in loops (Invoices, useAddSubscription, useCreateCreditNote)
- Set replaces includes() in loops (useFilters, ChargeFilter)
- flatMap replaces map().filter(Boolean) double-pass (useFieldError, TextInputFieldForTanstack, PricingBlockView)
- reduce replaces sort()[0] for max (getMostRecentPaymentMethodId)
- length check before every() (mapFromApiToForm)
- structuredClone replaces JSON parse/stringify (CustomerInvoiceRegenerate)
@ansmonjol ansmonjol self-assigned this Jul 1, 2026
@ansmonjol
ansmonjol marked this pull request as draft July 3, 2026 12:50
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