* fix(ci): point smoke test at core fixture
Co-authored-by: Ray Arayilakath <me@rayhanadev.com>
* style: format JSON smoke fixture path
Co-authored-by: Ray Arayilakath <me@rayhanadev.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
* refactor: collapse @react-doctor/types and @react-doctor/project-info into core
Eliminates two private workspace packages whose only role was to
namespace shared TS types and project-discovery helpers — neither
was a real consumer boundary, and the indirection cost a `workspace:*`
edge per leaf package without saving any code.
**Layout**
- `packages/types/` → `packages/core/src/types/` (10 files, pure TS types)
- `packages/project-info/` → `packages/core/src/project-info/`
(27 files, discoverProject + framework detection + monorepo helpers)
- `packages/types/src/react-native-dependency-names.ts` →
`packages/oxlint-plugin-react-doctor/src/react-native-dependency-names.ts`
(the canonical RN-aware-package list lives next to its heaviest
consumer — the rule gate — and is re-exported from core to avoid
the rule-plugin ↔ core dependency cycle the previous types package
was specifically designed to break).
The two packages are deleted from the workspace; every consumer
(`react-doctor`, `api`, `oxlint-plugin-react-doctor`, `actions/review`,
+130 source/test files) now imports from `@react-doctor/core` directly.
**Errors cleanup**
Dropped the legacy plain-class `ReactDoctorError` and `isReactDoctorError`
from `project-info`. The four narrow errors (`ProjectNotFoundError`,
`NoReactDependencyError`, `PackageJsonNotFoundError`,
`AmbiguousProjectError`) now extend `Error` directly — the only valid
hierarchy is the tagged `Schema.TaggedErrorClass` `ReactDoctorError`
from `core/errors.ts`, which has been the canonical one since the
Effect v4 refactor. Added `isProjectDiscoveryError` as a narrow type
guard for the four leaves.
The discovery errors are thrown synchronously BEFORE the Effect
runtime takes over (at `diagnose()` / CLI entry points); inside the
runtime, the Project service's `translateProjectInfoError` converts
each one to its tagged-reason equivalent.
**Schema vs type name collisions**
`schemas.ts` exports `Diagnostic` / `JsonReport` / `JsonReportMode` /
… as Schema.Class values, while `types/` exports the same names as
TS types. The core barrel re-exports the TS-type versions
(broad-strokes API) and exposes the Schema classes via a new
`@react-doctor/core/schemas` subpath export for the two callers
that need them (`Reporter` service, smoke validator, schema tests).
**Validation**
- `pnpm typecheck` ✓
- `pnpm lint` ✓
- `pnpm format` ✓
- `pnpm test` ✓ (1449 passing, 3 skipped — no regressions)
- `pnpm smoke:json-report` ✓
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(core): keep RN-dependency constants out of types barrel to avoid pulling oxlint plugin into discovery
Bugbot caught (medium severity): re-exporting `isReactNativeDependencyName` from
`oxlint-plugin-react-doctor` via the core types barrel meant every consumer of
`@react-doctor/core` (including `discoverProject` / `discoverReactSubprojects`)
would load the entire 286-rule plugin at module-init.
Adds a tiny standalone leaf at `core/src/project-info/internal-rn-dependency-names.ts`
(zero imports, just the constants the workspace-discovery gate needs) and switches
`is-package-json-react-native-aware.ts` to use it. The types barrel no longer
re-exports the plugin-side constants — rule consumers import them directly from
`oxlint-plugin-react-doctor`.
The duplication is intentional and documented at both files; the two lists are
short enough that a future regression test in the plugin can observe the union.
Validated: typecheck + test (1449 pass) + lint + format all green.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(scripts): escape leaderboard entries before writing them into README
`update-leaderboard.ts` interpolated `entry.name` and
`entry.githubUrl` directly into a markdown table that the workflow
later commits to the canonical README. A compromise of the upstream
benchmarks repo could land arbitrary markdown (or HTML that GitHub's
renderer accepts, like `<img>`) into the README and from there
onto npm via the published `react-doctor` package's README field.
Escape repo names against the markdown table grammar (backslash, pipe,
brackets, backtick, angle brackets, newlines), reject entries whose
`githubUrl` doesn't parse as an `https://github.com/...` URL, and
`encodeURI` the URL before substituting so a stray space or paren
can't terminate the markdown link target.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* fix(website): validate leaderboard github URLs to prevent XSS via href
The hosted leaderboard rendered `<a href={entry.githubUrl}>` with
no protocol check. `entry.githubUrl` is read from
`react-doctor-benchmarks` (a separate repo), so a compromise of
that repo could land a `javascript:` href on react.doctor. React
18+ no longer blocks `javascript:` URLs in href, so this was a
live XSS path even without a same-origin cookie story.
Parse each URL through the WHATWG URL constructor and only keep
entries whose protocol is `https:` and whose host is exactly
`github.com`. Filter happens at fetch time so every downstream
render uses the sanitized list.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
* fix(security): zip-slip + git-flag-injection defenses for the Effect-based services
Re-applies the two source-side security fixes from #428 onto the
current Effect v4 services (the original commits were authored
against the pre-refactor sync helpers in get-staged-files.ts /
get-diff-files.ts that no longer exist).
## 1. Zip-Slip: staged path traversal
`StagedFiles.materialize` was resolving each staged path with
`path.join(tempDirectory, relativePath)`, which happily expands
`..` segments outside the temp dir. A deliberately crafted index
entry (via `git update-index --add`, a malicious pack, or a
symlinked working tree) could write attacker-controlled files
anywhere the running user can write.
Added `isPathInsideDirectory(child, parent)` helper, resolved
`tempDirectory` up front, and the per-file loop now resolves the
candidate against the resolved temp dir and `continue`s when the
result falls outside. Same shape as the standard Zip-Slip defense.
## 2. CVE-2018-17456: git-flag injection via --diff <base>
`Git.diffSelection` was forwarding `explicitBaseBranch` to
`git rev-parse --verify <branch>` and `git merge-base <branch> HEAD`
unchanged. A value starting with `-` (e.g. `--upload-pack=evil`)
gets parsed by git as an option instead of a refname — the
CVE-2018-17456 shape.
Added `isSafeGitRevision(candidate)` at the top of the Git service
and a check at the start of `diffSelection`. The validation runs
BEFORE any subprocess spawn, so a malicious ref never reaches
`ChildProcess`. Rejects: empty, leading `-`, leading/trailing `.`,
embedded `..`, `@{` reflog suffix, or any character outside
`[A-Za-z0-9_./-]`.
In the canonical CI path the GitHub composite action already
guards with `case "$DIFF_BASE" in -* )` before the CLI is invoked,
so today's exposure is limited to local CLI runs where the user
is attacking themselves. Hardening the library boundary anyway
so other callers don't have to re-implement the check.
## Regression tests
- `Git.diffSelection — git-flag injection` (6 cases): leading dash,
`..` rev-range, `@{` reflog, shell metacharacters, leading dot,
empty. Uses `Git.layerNode` because validation runs before any
spawn, so the suite never reaches `git`.
- `StagedFiles.layerNode — Zip-Slip defense` (1 case): a staged
index containing `../escaped.ts` writes only the inside-the-temp
file and never touches the parent dir.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
First PR of the Effect v4 rewrite. Adds the architectural primitives
that subsequent PRs build on, while preserving every public contract
(inspect(), diagnose(), CLI flags, JSON schemaVersion: 1, GitHub Action).
## What lands
- packages/core/src/errors.ts — 9 leaf Schema.TaggedErrorClass
(OxlintUnavailable, OxlintBatchExceeded, OxlintSpawnFailed,
OxlintOutputUnparseable, ConfigParseFailed, ProjectNotFound,
NoReactDependency, AmbiguousProject, DeadCodeAnalysisFailed) composed
into ReactDoctorError. Helpers formatReactDoctorError /
isReactDoctorError / isSplittableReactDoctorError (all _tag-based,
zero string-grepping).
- packages/core/src/schemas.ts — Diagnostic + Severity + JsonReport
(Schema.Union for forward-compat with a future v2) +
buildDiagnosticIdentity. Schemas mirror the existing @react-doctor/types
interfaces; ProjectInfo / ScoreResult stay Schema.Unknown until PR 3.
- packages/core/src/refs.ts — Context.Reference for env-derived ambient
config (OxlintSpawnTimeoutMs, OxlintOutputMaxBytes,
StagedFilesTempDirPrefix). Tests override via Layer.succeed.
- packages/core/src/paths.ts — Schema.brand for OxlintBinaryPath +
NodeBinaryPath. Catches the swap at compile time.
- packages/core/src/constants.ts — hoisted OXLINT_SPAWN_TIMEOUT_MS from
the inline IIFE in run-oxlint.ts with explanatory docstring.
## Wiring
- handle-error.ts and build-json-report-error.ts dispatch to the
tagged-error message getter when isReactDoctorError(error), else
fall back to the existing formatErrorChain. run-oxlint.ts still
throws plain Errors; PR 2 converts it.
- scripts/smoke-json-report.ts runs the built CLI against
tests/fixtures/basic-react and Schema.decodeUnknownSync's the stdout
against the new JsonReport schema. New CI step (must stay green
through PR 8). Verified locally that a full --no-offline run with
263 real diagnostics decodes cleanly.
## Effect v4 deps
- effect@4.0.0-beta.70 in packages/core/dependencies and
packages/react-doctor/dependencies. Marked neverBundle in
packages/react-doctor/vite.config.ts (~1MB+ of tree-shakable source;
installers pull it as a regular dep — matches react-doctor-evals).
- @effect/vitest@4.0.0-beta.70 as a devDependency of core for PR 3+.
## Patterns
Every new file matches react-doctor-evals conventions exactly:
- import * as X from "effect/X" (never the umbrella import)
- Schema.TaggedErrorClass<Self>()("Tag", { fields }) with get message()
delegating to Cause.pretty(Cause.fail(this.cause)) for opaque causes
- Context.Reference<T>("react-doctor/X", { defaultValue }) with env-var
reads in defaultValue
- Schema.brand("X") via .pipe()
- kebab-case file names (per AGENTS.md)
## Validation
- pnpm typecheck — 10/10 tasks green
- pnpm test — 113 files, 1442 pass / 3 skipped (up from 1198; +244 new
tests in errors.test.ts and schemas.test.ts)
- pnpm lint — clean
- pnpm format:check — clean across 1119 files
- pnpm build — all 7 packages produce dist/
- pnpm smoke:json-report — schema-valid v1 JsonReport
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(react-doctor): forward reactMajorVersion in programmatic diagnose() + cleanups
H1 (regression introduced by #172) — `diagnose()` in `src/index.ts`
forgot to forward `reactMajorVersion` to `runOxlint`. After the
directional version-gating change in #172, that meant every
"prefer-newer-api" rule (today: `prefer-use-effect-event`) was silently
skipped for every programmatic API consumer, even on React 19+
projects. The CLI (`scan.ts`) was unaffected because it always passed
the version explicitly.
Fix is one line + one import — mirror what `scan.ts` already does.
Added `tests/diagnose.test.ts` with a regression test that asserts
`prefer-use-effect-event` fires on a React 19 fixture, plus a
symmetric guard that it stays skipped when the React version can't
be parsed (e.g. a github: range).
H2 — updated the stale docstring on `runOxlint`'s `reactMajorVersion`
field. The doc still claimed "`null` means unknown — leave those
rules enabled" but after #172 the null branch is directional
(deprecation-warning rules stay on, prefer-newer-api rules go off).
M1 — `SUB_HANDLER_DIRECT_CALLEE_NAMES` was just an alias of
`TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES`. Both names existed for
narrative reasons in different files; knip flagged the duplicate
export. Collapsed to the canonical name and updated the one consumer
(`isCallExpressionWithSubHandlerCallee` in state-and-effects).
L1 — moved `walkInsideStatementBlocks` from inline in
`state-and-effects.ts` to `plugin/helpers.ts` next to its sibling
`walkAst`. It already had four call sites and is the natural
"synchronous-only" walker for any rule asking what runs inside an
effect's own body — colocating with `walkAst` makes future rules
discover it.
L2 (audit finding) — proposed receiver-gating
`post`/`put`/`patch` in `EVENT_TRIGGERED_SIDE_EFFECT_CALLEES`
INTENTIONALLY NOT TAKEN. The canonical "You Might Not Need an Effect"
§6 example is `post(jsonToSubmit)` as a bare callee, so removing
those names breaks textbook detection (3 existing tests / fixtures).
Documented the trade-off in the constants.ts docstring.
Validation
- 642 tests passing (640 baseline + 2 new diagnose-API regressions)
- typecheck / lint / format clean
Co-authored-by: Cursor <cursoragent@cursor.com>
* chore: format scripts/benchmark-scores.ts
Pre-existing formatting issue introduced in 6afdc04 — the script was
committed unformatted, breaking `pnpm format:check` on every PR
branched off main since. Picked up by rebasing this PR.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Ray Arayilakath