Skip to content

fix(ci): harden react-doctor workflow and comment renderer#2525

Merged
pauldambra merged 1 commit into
mainfrom
posthog-code/react-doctor-hardening
Jun 8, 2026
Merged

fix(ci): harden react-doctor workflow and comment renderer#2525
pauldambra merged 1 commit into
mainfrom
posthog-code/react-doctor-hardening

Conversation

@pauldambra

@pauldambra pauldambra commented Jun 8, 2026

Copy link
Copy Markdown
Member

Problem

#2520 added the react-doctor CI action. A multi-perspective review of the equivalent change on posthog/posthog (PostHog/posthog#62084) surfaced a few reliability and robustness issues in the same workflow + renderer that were already merged here. This is the catch-up hardening.

Changes

react-doctor.yml:

  • Fail closed on git diff failure. The scan step runs set -uo pipefail without -e, so previously if git diff base...head failed (e.g. the base SHA was missing), the redirect left an empty changed-files list — indistinguishable from "no changes" — and the gate passed green without scanning anything. Now a failed diff exits 1.
  • Edit-safe exit-code capture. npx ... > "$REPORT"; status=$? on adjacent lines instead of echo "exit-code=$?", so the gate can't be silently broken by inserting a command between the run and the capture.

react-doctor-comment.mjs:

  • Cap the errors list at MAX_LISTED (50) like warnings already were, so a large error set can't exceed GitHub's 65536-char comment limit (which would 422 and post no comment). Named the shared cap.
  • Neutralise PR-controlled strings. Strip backticks and angle brackets from file paths, rule names, and titles so a crafted filename can't spoof the comment layout (e.g. inject a stray </details> or a fake "no issues" line). GitHub already sanitises for XSS; this prevents cosmetic spoofing.

No behaviour change for the normal (clean / has-findings) path.

How did you test this?

I'm an agent. Validated the workflow YAML parses, ran the renderer locally against synthetic reports including a malicious-input case (backtick/angle-bracket file path and </details><script> title) to confirm they're stripped, and ran Biome (biome ci) on the renderer. The workflow exercises itself on this PR.

Publish to changelog?

no


Created with PostHog Code

Follow-up hardening for the react-doctor action added in #2520, from a
multi-perspective review:

- fail closed when `git diff` fails, so a broken diff can no longer look
  like "no changed files" and pass the gate without scanning
- capture the scan exit code into a variable on the same line as npx so
  the gate is not silently broken by a future edit between the two lines
- cap the errors list (like warnings) to avoid exceeding GitHub's comment
  size limit, and name the shared cap MAX_LISTED
- strip backticks and angle brackets from PR-controlled strings so a
  crafted file path or rule name cannot spoof the comment layout

Generated-By: PostHog Code
Task-Id: ac09988a-6c71-4856-87d6-32b9e44b7684

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@pauldambra pauldambra marked this pull request as ready for review June 8, 2026 11:55
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 0f9835e.

@pauldambra pauldambra enabled auto-merge (squash) June 8, 2026 11:56
@pauldambra pauldambra requested a review from a team June 8, 2026 11:56

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f9835e9c6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/scripts/react-doctor-comment.mjs
@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/scripts/react-doctor-comment.mjs:29-32
The URL portion of the Markdown link still uses the raw `file` value. File paths that contain a `)` character — reasonably common in React codebases (e.g., `Button(deprecated).tsx`) — will cause GFM to close the link at the first unmatched `)`, producing a broken link and leaking the remainder as visible text. The display text is correctly sanitised by `inline(file)`, but the URL path is not.

```suggestion
const encodedFilePath = (file) =>
  file
    .split("/")
    .map((segment) => encodeURIComponent(segment))
    .join("/");
const fileLink = (file, line) =>
  slug && head
    ? `[\`${inline(file)}:${line}\`](${server}/${slug}/blob/${head}/${encodedFilePath(file)}#L${line})`
    : `\`${inline(file)}:${line}\``;
```

Reviews (1): Last reviewed commit: "fix(ci): harden react-doctor workflow an..." | Re-trigger Greptile

Comment thread .github/scripts/react-doctor-comment.mjs
@pauldambra pauldambra merged commit 3870c79 into main Jun 8, 2026
20 of 21 checks passed
@pauldambra pauldambra deleted the posthog-code/react-doctor-hardening branch June 8, 2026 12:06
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.

2 participants