mirror of
https://github.com/millionco/react-doctor.git
synced 2026-09-14 20:00:24 +08:00
v0.5.0
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aiden Bai
|
0a231000bb |
fix(security): four pre-existing findings from a deep audit (rebased onto Effect v4) (#435)
* 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> |
||
|
Aiden Bai
|
83a70de91e |
feat(core): Effect v4 foundation — tagged errors, schemas, refs, paths (#405)
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>
|
||
|
Aiden Bai
|
914e5f3aee |
feat: leaderboard in README + dedicated /leaderboard page (#176)
* docs(readme): add leaderboard section with top 10 from benchmarks repo Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com> * ci(leaderboard): refresh README leaderboard from benchmarks json on schedule Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com> * feat(website): add /leaderboard page driven by react-doctor-benchmarks Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com> * chore(scripts): drop bench:scores in favor of leaderboard.json Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com> * docs(readme): trim leaderboard table to repo + score Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com> * docs(readme): drop raw-results link from leaderboard section Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com> * fix(scripts): make update-leaderboard idempotent against formatter Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com> * refactor(website): extract score thresholds, color, label, doctor face to shared utils Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com> |
||
|
Aiden Bai
|
4fb4d27e85 |
fix(react-doctor): forward reactMajorVersion in programmatic diagnose() + cleanups (#174)
* 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
|
||
|
Nisarg Patel
|
6afdc04ba7 | chore(package): add benchmark scores script to package.json |