Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook

[justin808]

What

Ships an agent-facing guardrail for React Server Components (RSC) work in this repo, so the security and cross-request-isolation footguns surfaced in the recent RSC security audit don't silently regress. This is the framework-side half of a two-PR effort; the downstream app-installer (rake task that installs an RSC-usage skill + hook into consuming apps) is a follow-up PR.

Why

A focused RSC security audit (issues #4595, #4596, #4597) found the RSC stack solid, but identified a handful of invariants whose quiet regression would reintroduce XSS / cross-user-leak / auth footguns. This encodes those invariants where agents and reviewers will see them, and hard-locks the one mechanical invariant (inline-script escaping) with a test — per the "automate mechanical constraints, document judgment calls" principle.

Changes

• .agents/skills/rsc-guardrails/SKILL.md — the invariants RSC changes must not regress: payload/script escaping, no per-request module-level state on server-render paths, module-resolution allow-list, client-reachable-entrypoint auth, no secrets/PII in logs, and the node-renderer trust boundary. Includes a PR review checklist and a red-flags list. Auto-available via the existing .claude/skills → .agents/skills symlink.
• packages/react-on-rails-pro/tests/injectRSCPayload.test.ts — a regression test proving a user-controlled RSC payload chunk containing </script> / <!-- is neutralized (</\script, <\!--) and cannot break out of the inline hydration script.
• .claude/hooks/rsc-guardrails-check.sh + .claude/settings.json — a non-blocking PostToolUse (Edit|Write) warning when a Pro RSC source builds an inline <script>/HTML string outside the sanctioned escapeScript/createScriptTag helpers. Always exits 0; comment-line filtered; zero false positives on the current tree.

Verification

• injectRSCPayload.test.ts: 77/77 pass (React 19.2.7). The new case's assertions were independently confirmed against the real escapeScript output.
• Hook dry-run: silent on good / sanctioned (injectRSCPayload.ts) / non-RSC paths; warns on a hand-built <script> probe. shellcheck clean.
• prettier --check clean; settings.json valid JSON; branch-lint + markdown-links pass.

No user-facing runtime change; no CHANGELOG entry.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Security
  • Added new React Server Component guardrails guidance for the trusted-server to client boundary, including safe inline payload/script handling, nonce sanitization expectations, and stricter treatment of request-derived data.
• Checks
  • Introduced a non-blocking repository hook that scans edited files for risky inline script/HTML injection patterns and emits advisory warnings.
  • Enabled this scan during the PostToolUse step for file edits/writes.
• Tests
  • Added security-focused tests covering script/HTML breakout attempts to ensure payloads remain safely escaped while preserving underlying content.
• Documentation
  • Published the guardrails guidance as a dedicated skill document and added a review checklist.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds documented RSC security invariants, an advisory post-edit scan for unsafe script sinks, hook wiring, and regression coverage for escaping </script> and <!-- sequences in injected payloads.

Changes

RSC security guardrails

Layer / File(s)	Summary
RSC guardrail contract .agents/skills/rsc-guardrails/SKILL.md	Documents script escaping, request isolation, module allow-lists, authentication, log redaction, renderer boundaries, review checks, and red flags.
Advisory guardrail scan .claude/hooks/rsc-guardrails-check.sh, .claude/settings.json	Scans selected RSC and streaming sources for raw script or HTML sinks and invokes the non-blocking scan after Edit and Write actions.
Hook and payload regression coverage packages/react-on-rails-pro/tests/injectRSCPayload.test.ts	Tests scan matching, exclusions, deduplication, sanctioned script-builder shapes, and neutralization of dangerous RSC payload sequences.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ClaudeCode
  participant GuardrailHook
  participant EditedFile
  ClaudeCode->>GuardrailHook: PostToolUse Edit/Write file path
  GuardrailHook->>EditedFile: Scan script and HTML sink patterns
  GuardrailHook-->>ClaudeCode: Emit advisory warning or no warning

Loading

Possibly related PRs

• shakacode/react_on_rails#4379: Both changes cover injectRSCPayload streamed script and payload behavior.
• shakacode/react_on_rails#4477: Both changes address the injectRSCPayload inline RSC script surface.
• shakacode/react_on_rails#4549: Both changes overlap on execution and coverage of the injectRSCPayload test suite.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: a guardrails skill, an escaping regression test, and an advisory hook.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg/rsc-agent-guardrails-skill

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR adds RSC guardrails for agents and tests. The main changes are:

• New rsc-guardrails skill documenting RSC security invariants.
• New non-blocking Claude hook for risky inline script and HTML patterns.
• New payload escaping test for </script> and <!-- in RSC chunks.

Confidence Score: 5/5

This looks safe to merge with a small guardrail coverage cleanup.

• The payload escaping test follows the production stream injection path.
• The hook is advisory and non-blocking.
• The hook can miss Ruby RSC streaming script changes because its scan scope is TypeScript-only.

.claude/hooks/rsc-guardrails-check.sh

Security Review

The new escaping test improves coverage for an XSS-sensitive RSC path. The advisory hook has one coverage gap: it does not scan Ruby RSC streaming script emitters.

Important Files Changed

Filename	Overview
.agents/skills/rsc-guardrails/SKILL.md	Adds agent-facing RSC guidance for escaping, request isolation, module resolution, auth boundaries, logging, and renderer trust boundaries.
.claude/hooks/rsc-guardrails-check.sh	Adds a non-blocking advisory hook for risky inline script and HTML patterns, with a narrow TypeScript-only scan scope.
.claude/settings.json	Registers the new advisory hook after Edit and Write tool use.
packages/react-on-rails-pro/tests/injectRSCPayload.test.ts	Adds a regression test for escaping script-breaking sequences in streamed RSC payload chunks.

_{Reviews (1): Last reviewed commit: "Add rsc-guardrails skill, payload-escapi..." | Re-trigger Greptile}

[github-actions]

size-limit report 📦

Path	Size
react-on-rails/client bundled (gzip)	63.54 KB (0%)
react-on-rails/client bundled (gzip) (time)	63.54 KB (0%)
react-on-rails/client bundled (brotli)	54.55 KB (0%)
react-on-rails/client bundled (brotli) (time)	54.55 KB (0%)
react-on-rails-pro/client bundled (gzip)	64.92 KB (0%)
react-on-rails-pro/client bundled (gzip) (time)	64.92 KB (0%)
react-on-rails-pro/client bundled (brotli)	55.82 KB (0%)
react-on-rails-pro/client bundled (brotli) (time)	55.82 KB (0%)
registerServerComponent/client bundled (gzip)	135.42 KB (0%)
registerServerComponent/client bundled (gzip) (time)	135.42 KB (0%)
registerServerComponent/client bundled (brotli)	81.72 KB (0%)
registerServerComponent/client bundled (brotli) (time)	81.72 KB (0%)
wrapServerComponentRenderer/client bundled (gzip)	127.87 KB (0%)
wrapServerComponentRenderer/client bundled (gzip) (time)	127.87 KB (0%)
wrapServerComponentRenderer/client bundled (brotli)	74.88 KB (0%)
wrapServerComponentRenderer/client bundled (brotli) (time)	74.88 KB (0%)

[coderabbitai]

🧹 Nitpick comments (1)

.claude/hooks/rsc-guardrails-check.sh (1)

26-27: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Use absolute path for grep to avoid cwd dependency.

grep on line 44 uses "$REL_PATH" (relative), but if the hook is invoked from a working directory other than REPO_ROOT, the file won't be found and the check silently skips. Since REPO_ROOT is already resolved, grepping "$FILE" (absolute) would be more robust.

♻️ Proposed fix

-MATCHES="$(grep -nE "$PATTERN" "$REL_PATH" 2>/dev/null | grep -vE '^[0-9]+:[[:space:]]*(\*|//|/\*)' || true)"
+MATCHES="$(grep -nE "$PATTERN" "$FILE" 2>/dev/null | grep -vE '^[0-9]+:[[:space:]]*(\*|//|/\*)' || true)"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/hooks/rsc-guardrails-check.sh around lines 26 - 27, Update the grep
invocation in rsc-guardrails-check.sh to use the absolute FILE path instead of
REL_PATH, ensuring the check works regardless of the hook’s current working
directory. Keep REL_PATH for any other purposes where the repository-relative
path is required.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.claude/hooks/rsc-guardrails-check.sh:
- Around line 26-27: Update the grep invocation in rsc-guardrails-check.sh to
use the absolute FILE path instead of REL_PATH, ensuring the check works
regardless of the hook’s current working directory. Keep REL_PATH for any other
purposes where the repository-relative path is required.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2bcbd0c9-0e83-45f9-b0e5-c955cbbc0a06

📥 Commits

Reviewing files that changed from the base of the PR and between 4747499 and b572036.

📒 Files selected for processing (4)

• .agents/skills/rsc-guardrails/SKILL.md
• .claude/hooks/rsc-guardrails-check.sh
• .claude/settings.json
• packages/react-on-rails-pro/tests/injectRSCPayload.test.ts

[claude]

Review

Overview. This PR adds a purely agent/reviewer-facing RSC security guardrail package: a new SKILL.md documenting six invariants for the trusted-server/untrusted-client RSC boundary, a regression test locking down escapeScript's </script>/<!-- neutralization, and a non-blocking PostToolUse shell hook that warns (never fails) when Pro RSC source files hand-build inline <script>/HTML instead of using the sanctioned helpers. No runtime code changes; scope matches the stated goal well.

Regression test (injectRSCPayload.test.ts)

• Verified against escapeScript/createScriptTag in packages/react-on-rails-pro/src/injectRSCPayload.ts: the regex (/<!--/g → <\!--, /<\/(script)/gi → </\$1) matches the test's expectations exactly, including the "escaped, not dropped" assertion (alert(document.cookie) still present) and the "no live breakout" assertion.
• Follows the existing setupTest/createMockRSCStream/collectStreamData conventions in the file precisely — good consistency.
• No issues found here; this is a solid, well-targeted regression test.

SKILL.md

• The invariants are well-reasoned and each maps to a concrete, checkable pattern (good "automate mechanical / document judgment" split).
• Issue (left inline): the "Reference" section's pointer for invariant Add linting and CI scripts #3 (module-resolution allow-list) cites a react_on_rails_rsc package with clientReferences.ts / flight-server.node*.ts — none of these exist anywhere in the repo. I confirmed the actual package list (react-on-rails, react-on-rails-pro, react-on-rails-pro-node-renderer, create-react-on-rails-app) and repo-wide searches for clientReference/flight-server come up empty. All the other file references in this section (sanitizeNonce.ts, RSCRequestTracker.ts, RSCProvider.tsx, RSCPrefetchStore.ts, the node-renderer worker/auth/config files, json_output.rb's ERB::Util.json_escape) checked out fine — this one line is the exception and should be fixed so the doc doesn't send a reader to a dead end on one of its most security-relevant invariants.

Hook (rsc-guardrails-check.sh)

• Correctly non-blocking (set -euo pipefail throughout, but every failure path is defused with || true / explicit exit 0), scoped narrowly to packages/react-on-rails-pro/src/*.ts(x), and exempts the sanctioned emitter files.
• Minor robustness nit (left inline): it validates file existence via $FILE but then greps $REL_PATH (relative to repo root). If ever invoked with a cwd other than the repo root, this fails silently (no warning, no error) — safe-by-default given it's advisory, but it means the guardrail can quietly never fire with no signal. Suggest grepping $FILE directly.
• Pattern quoting ('['"'"'"]<script|...'`) is correct but cryptic — a one-line comment explaining the character class (backtick/single/double quote) would help future maintainers avoid "fixing" it into a bug.

Other

• settings.json change is minimal and additive (new hook entry appended, unrelated to autofix-file).
• No CHANGELOG entry is correct per project policy (agent-tooling/tests aren't user-visible changes).
• No security concerns with the hook itself — it only reads a file and writes to stderr; it can't be leveraged as an injection vector since it doesn't eval/execute file contents.

Overall: solid, low-risk addition. The one broken file reference in SKILL.md is worth fixing before merge since it undermines the credibility of the exact invariant (module-resolution allow-list) that's hardest to verify by inspection.

[claude]

Review

Overview: Small, low-risk PR adding agent-facing tooling for RSC security: a SKILL.md documenting six invariants (script escaping, no per-request module state, module-resolution allow-list, entrypoint auth, no secrets in logs, node-renderer trust boundary), a non-blocking PostToolUse shell hook that greps for hand-built inline <script>/HTML sinks in react-on-rails-pro/src, and a new regression test on injectRSCPayload. No runtime/production code changes.

Correctness

• Verified the new test's assertions against the actual escapeScript implementation (packages/react-on-rails-pro/src/injectRSCPayload.ts:44-46): escapeScript replaces <!-- → <\!-- and </script (case-insensitive) → </\script. The test's userControlledFlightChunk runs through JSON.stringify (which does not escape /) then escapeScript, so the assertions (toContain('</\\script'), toContain('<\\!--'), not.toContain the raw breakout string) are all consistent with the real code path — this is a solid, non-tautological regression test.
• The hook script's case "$FILE" in */packages/react-on-rails-pro/src/*.ts ... correctly matches nested files too (bash case pattern matching, unlike filename globbing, lets * span /), so files like src/registerServerComponent/server.rsc.ts or src/cache/RedisCacheHandler.ts are still covered — confirmed this isn't a gap.
• settings.json diff is a clean additive change (new PostToolUse command alongside the existing autofix-file hook); valid JSON, no ordering concerns since the hook always exits 0.

Minor nits (posted inline on the hook script)

• grep -nE "$PATTERN" "$FILE" could misinterpret a file path starting with - as a flag; grep -nE -- "$PATTERN" "$FILE" closes it for free, low risk given $FILE is agent-supplied.
• If jq is missing, the hook silently no-ops rather than signaling that the guardrail didn't run — acceptable for an advisory-only hook, but worth a one-line comment.

Coverage/quality

• The line-based grep heuristic (plus comment-line filtering) will miss anti-patterns spread across multiple lines (e.g. a multi-line template literal opening <script), but that's an inherent and reasonable limitation for a fast advisory hook, not a blocker.
• Good use of escapeRegExpLiteral-adjacent care elsewhere in the file; the new test doesn't introduce any regex/escaping mismatches.

Security: No new attack surface — this PR only adds documentation, an advisory dev-tool hook, and a test that locks in existing escaping behavior. No secrets, no new runtime code paths.

Overall: solid, well-scoped addition. No blocking issues found.

[justin808]

Address-review summary

Scan scope: full review history through final head c0739f7672598dc44e8989ef9dabef78a96c01c6, based on 6325e108efbf1d2e6eeed8b9c9c4753fe6841d6f after two non-destructive main-sync merges.

Mattered

• Added the missing Ruby RSC stream guardrail and documented its sanctioned Rails JSON/HTML escaping path.
• Corrected the external react-on-rails-rsc package/runtime reference and reverified the absolute-path behavior.
• Made successful PostToolUse warnings visible through structured additionalContext without exposing edited source text.
• Replaced file-wide emitter exemptions with focused source-line exclusions. Raw-HTML sinks now have an independent scan with no allowlist.
• Limited the sanctioned createScriptTag shape to the exact owning packages/react-on-rails-pro/src/injectRSCPayload.ts path, including a nested-shadow-owner regression.
• Extended innerHTML detection to simple, arithmetic/bitwise/shift, logical, and computed assignments while explicitly excluding equality checks.
• Extended the same assignment coverage to direct and computed outerHTML sinks.
• Narrowed dangerouslySetInnerHTML detection to JSX assignment and object-property sink forms, with negative controls for reads, function forwarding, object shorthand forwarding, and equality checks.

Optional or duplicate signal

• Added grep -- safety, preserved a readable no-jq manual fallback, simplified the script-tag scan, and deduplicated overlapping matches.
• Eight late optional/duplicate review comments were explicitly [auto-deferred] before resolution: exact-line allowlist refactoring, /bin/bash portability, moving hook tests to a new file, and deeper real-no-jq subprocess coverage. These would add scope or post-candidate churn without changing the verified correctness outcome.
• Status-only comments, review summaries duplicating inline findings, and metadata-only workflow comments were recorded as non-actionable evidence.

Thread disposition

All 18 exact review threads received evidence-based replies before resolution. Four late correctness findings (compound assignment coverage, equality false positives, missing outerHTML coverage, and broad dangerouslySetInnerHTML false positives) were fixed test-first; eight late optional/duplicate findings received explicit autonomous-defer replies. Unresolved-thread count after final normalized refresh: 0.

Late-thread roots:

• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)
• Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook #4599 (comment)

Verification

• Test-driven late fixes: each compound/equality, nested-owner, logical-assignment, computed-assignment, exponentiation, outerHTML, and dangerouslySetInnerHTML ownership gap was observed RED for the intended reason before the implementation turned it GREEN.
• pnpm --filter react-on-rails-pro exec jest tests/injectRSCPayload.test.ts --runInBand — 91/91 passed on the exact final head (also independently rerun by the final reviewer).
• pnpm --filter react-on-rails-pro type-check — passed on the exact final head.
• Full ESLint and Prettier checks passed; the final reviewer also reran targeted Prettier successfully.
• script/check-pro-license-headers — all 860 in-scope Pro files passed.
• bash -n, ShellCheck, git diff --check, and pre-push branch/link checks — passed.
• .agents/bin/validate --changed passed dependency installation, docs checks, both RuboCop passes, and the package build before the OS killed the ESLint process with signal 9. Direct full pnpm run lint completed green, and the validator emitted no ESLint diagnostic.
• Independent exact-head review receipt: codex review --base origin/main, gpt-5.6-sol/xhigh, session 019f6b10-5b55-7550-981e-18206a88bac6, exited 0 with no findings on c0739f7672598dc44e8989ef9dabef78a96c01c6.

Outcome

• Review-fix commits: 3470143b0183199a1a04e1162ee178a1fbcdc3c8, 78b2b95b70bc200baec34f1b6658d7a7d6b30ea7, 9a15d3338e2aecab95c8783c3cc1f4fab714c103, 26e4a867782bd7e779001bd639f2e22f28cd57c3, c0739f7672598dc44e8989ef9dabef78a96c01c6.
• Base-sync merges: e9a381349e7431f3c9698a1a1f57613019c5a993, acedaa071e52c3743a1be821829d04e0ba20d800.
• Final branch diff remains exactly four owned paths: the RSC guardrail skill, Claude hook, Claude settings, and Pro injectRSCPayload test.
• Deferred tracking: none. Maintainer decisions required: none (decision_count: 0).
• Review churn: five consolidated review-fix commits, two non-destructive base-sync merges, and no force-pushes.
• Current-head required CI readiness reports READY, but broader hosted checks and the configured Claude review are still running on c0739f7672598dc44e8989ef9dabef78a96c01c6; the lane remains waiting-on-checks-or-review for coordinator/QA closeout.

[justin808]

+ci-status

[justin808]

+ci-run-hosted

[github-actions]

CI Status

Head SHA: 3470143b0183
Changed files: 4
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: absent
force-full-hosted-ci label: absent
Current hosted-CI waiver: not present for this SHA

Only the required gate is active unless hosted CI is requested.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3470143b01

ℹ️ 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".

[github-actions]

Hosted CI Requested

Triggered 9 workflow(s) for 3470143b0183.
Mode: optimized hosted CI (path-selected by script/ci-changes-detector).
Added ready-for-hosted-ci, so future commits will keep running optimized hosted CI until +ci-stop-hosted is used.

View progress in the Actions tab.

[claude]

Review

Solid, low-risk addition: an agent-facing RSC security skill, a real regression test for the escapeScript XSS-breakout invariant, and a non-blocking advisory hook. I verified the source claims against the current tree — all files/functions referenced in SKILL.md exist, sanitizeNonce is genuinely called before the nonce lands in the <script> attribute (injectRSCPayload.ts:1639), and escapeScript's regex (</(script)/gi, <!--) matches what the new test asserts.

Code quality / correctness

• packages/react-on-rails-pro/tests/injectRSCPayload.test.ts: the new test is well constructed and consistent with existing helpers (setupTest, createMockRSCStream, expectedPayloadPushScript). It correctly asserts both that the unescaped breakout string never appears and that the payload content (alert(document.cookie)) survives inertly — good, because a test that only checks "doesn't contain the bad string" could pass by silently dropping data instead of escaping it.
• .claude/hooks/rsc-guardrails-check.sh: clean, defensive shell (set -euo pipefail, -- before grep patterns, safe jq --arg usage — no injection risk from the file path or line content it prints). Always exits 0 as documented, so it can't block a session.
• The case "$FILE" in */packages/react-on-rails-pro/src/*.ts ...) pattern does correctly match nested subdirectories (e.g. src/registerServerComponent/*.ts) — case/fnmatch-style glob matching, unlike pathname expansion, lets * span /. Confirmed this isn't a coverage gap.

Coverage gap worth noting

The hook (and the only automated test) cover invariant 1 only (inline script/HTML escaping) for packages/react-on-rails-pro/src/** plus one hardcoded Ruby file. Invariants 2–6 in SKILL.md — per-request module state, module-resolution allow-list, auth on render entrypoints, secret/PII logging, and especially invariant 6, the node-renderer RCE trust boundary — have no automated or hook backing at all. packages/react-on-rails-pro-node-renderer/src/** (explicitly named as the highest-severity surface in the skill doc) isn't touched by the hook's case filter, so an agent could remove the production-password check or widen the bind address with no mechanical pushback, only relying on the agent reading the skill doc. This is called out in the PR description as an intentional "automate mechanical constraints, document judgment calls" tradeoff, so not a blocker, but worth a follow-up issue if the node-renderer auth invariant is high-value enough to deserve at least a targeted grep-based check (e.g. flagging edits to authHandler.ts/worker.ts that touch the password/bind-address logic).

Minor nit

The comment-line filter in the hook (grep -vE '^[0-9]+:[[:space:]]*(\*|//|/\*)') only strips full-line comments. A match on a trailing inline comment (const x = 1; // dangerouslySetInnerHTML example) would still surface a warning. Harmless given the hook is advisory/non-blocking, just a minor source of noise.

Everything else

• .claude/settings.json diff is a minimal, correctly-formed addition to the existing PostToolUse hook array; ordering after autofix-file is fine since the guardrail script re-reads the file from disk rather than relying on stale content.
• No user-facing runtime change, so skipping the CHANGELOG entry is correct per .claude/docs/changelog-guidelines.md ("user-visible changes only").
• Didn't find any security issues in the new shell script or test code itself.

Overall: good, well-scoped defensive tooling. No blocking issues.

[justin808]

+ci-status

[justin808]

+ci-run-hosted

[github-actions]

CI Status

Head SHA: 3470143b0183
Changed files: 4
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: present
force-full-hosted-ci label: absent
Current hosted-CI waiver: not present for this SHA

Optimized hosted CI is enabled for this PR.

[github-actions]

Hosted CI Requested

Triggered 9 workflow(s) for 3470143b0183.
Mode: optimized hosted CI (path-selected by script/ci-changes-detector).
Added ready-for-hosted-ci, so future commits will keep running optimized hosted CI until +ci-stop-hosted is used.

View progress in the Actions tab.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 26e4a86778

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0739f7672

ℹ️ 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".

[claude]

Review Summary

Overview: Adds an agent-facing rsc-guardrails skill documenting five RSC security/isolation invariants (payload/script escaping, no per-request module state, module-resolution allow-list, entrypoint auth, no secrets in logs, Node-renderer trust boundary), a regression test proving escapeScript neutralizes </script>/<!-- breakout in RSC payload chunks, and a non-blocking PostToolUse hook that warns (never blocks) when Pro RSC source hand-builds inline <script>/raw-HTML strings outside the sanctioned helpers. Purely additive — no production runtime code changed.

Strengths

• The new injectRSCPayload.test.ts case is a genuine, well-targeted security regression test: it exercises a realistic breakout payload (</script><script>...) and asserts both that the dangerous unescaped form never appears and that the data survives escaping (not silently dropped).
• SKILL.md is concrete and actionable — each invariant names an owner file, a "why," and links back to the audit issues ([Security] RSC payload endpoint renders arbitrary components with attacker-controlled props and no built-in authorization #4595–[Security] Secret/PII hygiene: server-render props & renderer_url reach logs/error trackers; misc Ruby hardening #4597) rather than being generic advice.
• The hook is carefully designed to fail open: set -euo pipefail combined with explicit || true fallbacks, exit 0 on any missing tool/file, and it never blocks a tool call.
• Good test coverage of the hook's own regex edge cases (compound/logical assignment operators, computed member access, comment-line exclusion, collision/dedup between the script-scan and sink-scan).

Issues raised inline

1. Confusing warning text (.claude/hooks/rsc-guardrails-check.sh:97) — the Ruby-file warning tells editors to "keep Ruby stream specs aligned with the TypeScript browser-performance-mark helper," which is unrelated to the escaping invariant being enforced and looks like a copy-paste artifact.
2. Brittle exact-line safe-listing (.claude/hooks/rsc-guardrails-check.sh:111-114) — the exemptions for injectRSCPayload.ts's sanctioned lines match the entire source line verbatim. Any future reformat/rename in that file will silently break the exemption and start producing false-positive warnings on the one file the hook is meant to trust.
3. Test file scope creep (packages/react-on-rails-pro/tests/injectRSCPayload.test.ts:256) — ~15 it() cases testing the bash hook script (each spawning a real bash subprocess via execFileSync) were added to a file otherwise dedicated to unit-testing injectRSCPayload. Suggest a separate test file so scope and CI cost are clearer.

Other observations (non-blocking)

• The hook computes git rev-parse --show-toplevel and does file I/O for every Edit|Write tool call repo-wide (not just RSC files) before its path filter short-circuits — minor added latency on every edit, everywhere, in perpetuity. Probably fine given it's cheap, but worth knowing it's not free.
• Hook input parsing diverges from the existing convention in .claude/hooks/main-ci-status-on-push.sh (that hook hard-requires jq and exits if absent; this one adds a manual $1 fallback for local testing). Not wrong, just inconsistent style within .claude/hooks/.
• No security concerns with the change itself — it only adds documentation, an advisory (non-blocking) hook, and a test; nothing in the production request path changes.

Overall: solid, low-risk addition. The three inline points are polish/maintainability nits rather than blockers.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2d857feaa

ℹ️ 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".

[claude]

Review: Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook

Overall this is a well-scoped, low-risk PR: it adds documentation and tests, and a non-blocking advisory hook — no production runtime code changes. I verified the diff (gh pr diff 4599) against the checked-out tree.

What it does

• .agents/skills/rsc-guardrails/SKILL.md: documents 6 RSC security/isolation invariants with a PR checklist and red-flag list.
• packages/react-on-rails-pro/tests/injectRSCPayload.test.ts: new regression test proving escapeScript neutralizes </script> and <!-- breakout attempts in user-controlled RSC payload chunks.
• .claude/hooks/rsc-guardrails-check.sh + .claude/settings.json: a new PostToolUse hook that greps edited Pro RSC source (and the Ruby stream emitter) for hand-built <script>/raw-HTML sinks and emits an advisory (always exits 0) warning.
• packages/react-on-rails-pro/tests/rscGuardrailsHook.test.ts: exercises the shell hook directly via execFileSync against fixture files.

Verification performed

• Confirmed escapeScript in packages/react-on-rails-pro/src/injectRSCPayload.ts matches the SKILL.md description (</script and <!-- are backslash-neutralized).
• Confirmed the new regression test reuses existing test helpers (createMockRSCStream, expectedPayloadPushScript, etc.) correctly rather than duplicating setup.
• Spot-checked every file path referenced in SKILL.md's "Reference" section (RSCRequestTracker.ts, RSCProvider.tsx, RSCPrefetchStore.ts, the node-renderer worker/auth files, sanitizeNonce.ts, stream.rb, json_output.rb, helper.rb) — all resolve to real files, so the doc isn't citing stale/renamed paths.
• Traced through the hook script's bash logic (quoting, pipefail interaction with grep's exit-1-on-no-match via the || true guards, the case glob matching which does match across / since case/[[ ]] use fnmatch without FNM_PATHNAME) — didn't find a correctness bug. The regex exclusions for equality checks (===), type-only dangerouslySetInnerHTML declarations, and destructuring aliases all check out against the stated test cases.
• Confirmed the CHANGELOG.md omission is correct per .claude/docs/changelog-guidelines.md ("Do NOT add entries for: linting, formatting, refactoring, tests, or documentation fixes").
• Compared against the existing bin/claude-hooks/autofix-file PostToolUse hook, which already pays the same cat/jq/git rev-parse --show-toplevel cost on every Edit/Write and additionally shells out to Prettier/RuboCop — so this new hook's added per-edit overhead (pure grep, no external formatter) is comparatively minor and consistent with existing precedent.

Observations (non-blocking)

1. Intentional brittleness, called out in the code itself: the SAFE_LAST_INDEX_LINE/SAFE_DOUBLE_ESCAPE_PATTERN_LINE/SAFE_SCAN_OVERLAP_LINE/SAFE_CREATE_SCRIPT_TAG_LINE constants in rsc-guardrails-check.sh are byte-exact copies of specific lines in injectRSCPayload.ts. Any future reformatting of that file (e.g. a Prettier print-width change) will silently break the exemption and produce a false-positive warning. The PR's own comment explains this tradeoff (false positive > false negative for a security guardrail), which I think is the right call for a non-blocking hook — just flagging it so it's not a surprise the next time injectRSCPayload.ts gets reformatted.
2. Coverage is intentionally narrow: the hook only pattern-matches invariant 1 (script/HTML escaping) in packages/react-on-rails-pro/src/** and the one Ruby stream file. Invariants 2–6 (module-scope state, module-resolution allow-list, auth boundaries, log redaction, node-renderer trust boundary) are documentation-only with no mechanical check — consistent with the PR description's stated "automate mechanical constraints, document judgment calls" principle, so this looks deliberate rather than an oversight.
3. Nice test coverage in rscGuardrailsHook.test.ts — compound/logical/computed assignment operators, type-only declarations vs. runtime sinks, destructuring aliases, and same-line dedup are all exercised, which gives confidence the regex logic won't regress silently.

No blocking issues found. Nothing here affects production runtime behavior (docs + tests + advisory-only local dev tooling).

[claude]

Review: Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook

Overview

Solid, well-scoped tooling PR: an agent-facing skill doc (SKILL.md) codifying the RSC trust-boundary invariants, a security regression test for escapeScript's </script>/<!-- neutralization, and a non-blocking PostToolUse bash hook that heuristically flags hand-built inline <script>/raw-HTML sinks in packages/react-on-rails-pro/src/**/*.ts(x). No runtime code changes — pure dev-tooling + tests.

Strengths

• The new injectRSCPayload.test.ts case is a real regression guard: it verifies both that the unescaped form (</script><script>alert(...)</script>) never appears in output, and that the actual escapeScript neutralization (</\script, <\!--) is present while the payload content itself survives — a well-constructed breakout test.
• emit_warning() uses jq -n --arg to build the hook's JSON output rather than string interpolation, avoiding injection/escaping bugs in the advisory message.
• The hook is carefully designed to fail open: always exits 0, and every risky operation ([ -z "$FILE" ], missing jq, missing file) degrades gracefully rather than blocking the edit.
• I checked all four SAFE_*_LINE exact-match constants in rsc-guardrails-check.sh against the current injectRSCPayload.ts source — they match byte-for-byte today, consistent with the PR's "zero false positives on the current tree" claim.
• rscGuardrailsHook.test.ts has genuinely good coverage of the regex edge cases (compound/logical/computed assignment operators, type-only dangerouslySetInnerHTML declarations, destructuring aliases, same-line collisions between sanctioned and unsanctioned patterns).

Issues / Suggestions

1. Hook coverage is narrower than the skill's stated scope (minor gap). rsc-guardrails-check.sh only scans packages/react-on-rails-pro/src/*.ts(x) (plus the one hardcoded Ruby stream.rb path). But SKILL.md's invariants explicitly reference other RSC-adjacent surfaces — packages/react-on-rails/src/sanitizeNonce.ts, the node-renderer worker/vm/authHandler files, json_output.rb/helper.rb. A hand-built script/HTML sink introduced in any of those files (e.g. the node-renderer package) would silently bypass the advisory check even though the skill calls them out as in-scope. Worth a follow-up to either broaden the path filter or note explicitly in the skill/hook comments that automation is intentionally limited to the one package.

2. Fragile exact-string dependency (self-acknowledged, but worth flagging for reviewers). The SAFE_*_LINE constants in the hook must be byte-for-byte copies of specific injectRSCPayload.ts lines. Since bin/claude-hooks/autofix-file (prettier/eslint --fix) runs before this hook in PostToolUse, any future reformatting of those lines (e.g. a prettier version bump changing wrap width) will silently break the exemption and start producing false-positive warnings on the sanctioned code every time it's touched. This is documented in the script's own "MAINTENANCE CONTRACT" comment, so it's a known tradeoff — but a more robust anchor (e.g. a // rsc-guardrails: sanctioned sentinel comment on the line, or matching by function name rather than exact text) would be less brittle than raw string equality.

3. Hardcoded /bin/bash in the test runner (rscGuardrailsHook.test.ts): execFileSync('/bin/bash', [rscGuardrailsHook, fixturePath], ...) assumes bash lives at that exact path. The script itself declares #!/usr/bin/env bash, so invoking it directly (execFileSync(rscGuardrailsHook, [fixturePath], ...)) or resolving bash via PATH would be more portable for contributors on systems where bash isn't at /bin/bash (e.g. some Nix/BSD setups). Low severity since CI is presumably Ubuntu, but worth considering for local-dev portability.

4. Nit: the new rscGuardrailsHook.test.ts carries the React on Rails Pro commercial-license header, consistent with every other file in packages/react-on-rails-pro/tests/, but the file it actually tests (.claude/hooks/rsc-guardrails-check.sh) is a repo-wide dev-tooling script with no such header and no Pro-exclusive logic. Not a functional problem, just worth double-checking this is the intended home for it rather than a .claude/hooks/ or top-level test location.

Security

• No security concerns with the hook itself: no eval, all file paths are safely quoted, JSON parsing goes through jq -r ... // empty rather than manual parsing, and it never executes content from the scanned file.
• The regression test correctly encodes the actual security property (breakout prevention), not just an implementation-detail snapshot.

Test coverage

Good — both the payload-escaping regression and the hook's regex behavior are covered with edge cases. No blocking gaps found.

Overall: a well-executed, low-risk, advisory-only addition. The two items worth a maintainer's attention before merge are #1 (coverage gap vs. the skill's stated scope) and #2 (brittle exact-line matching interacting with the autofix hook ordering) — neither is a blocker given the hook is non-blocking and well-tested, but both are worth a conscious "yes, we accept this" rather than being missed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cf059d2a86

ℹ️ 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".

[claude]

Review summary

This PR ships an advisory-only, well-tested piece of agent tooling: an RSC-guardrails skill doc, a payload-escaping regression test, and a non-blocking PostToolUse shell hook. Overall quality is high.

Strengths

• injectRSCPayload.test.ts: the new breakout-escaping test is well-constructed — it asserts both that the unescaped form never appears and that the escaped </\script / <\!-- markers do, and that the underlying data survives. Cross-checked escapeScript/createScriptTag in packages/react-on-rails-pro/src/injectRSCPayload.ts directly; the claims in SKILL.md about what it neutralizes are accurate.
• .claude/hooks/rsc-guardrails-check.sh: despite the complexity (multiline sink detection, parser-only-call vs. real-sink disambiguation, exact-line allowlisting for the sanctioned createScriptTag shape), I traced through the trickier cases (compound assignments, === vs =, prettier-split dangerouslySetInnerHTML, same-line collisions between a parser-only .replace() and a real emission) and they check out against the accompanying rscGuardrailsHook.test.ts suite. The set -e/pipefail handling around every grep pipeline is careful (each fallible pipeline is guarded with || true), and the exact-string allowlist constants for injectRSCPayload.ts currently match the real source, so there's no drift yet.
• The hook is genuinely non-blocking (always exits 0) and is honest about that in its own comments — it's a nudge, not a gate.

Gap worth addressing (left as inline comments)

• The skill's invariant-1 "owner file" list and the hook's file glob are both scoped to packages/react-on-rails-pro/src/* (+ one Ruby file). But the base package has its own equivalent of escapeScript: packages/react-on-rails/src/scriptSanitizedVal.ts, used via wrapInScriptTags for the console-replay inline <script> tag, and explicitly relied on in react_on_rails/lib/react_on_rails/helper.rb (~line 740) before an html_safe content_tag(:script, ...). It's the same trusted-server/untrusted-client boundary this PR documents, but it's absent from SKILL.md's reference list and unscanned by the new hook. It's also weaker than escapeScript — it only neutralizes </script-like sequences, not <!--. Worth deciding whether to fold it into this guardrail (reference + hook glob) or explicitly note it's out of scope and why.

No other correctness, security, or performance issues found. This is documentation/tooling-only with no runtime behavior change, so the "no CHANGELOG entry" call is reasonable.

[claude]

Review summary

This PR ships documentation + tooling for RSC security guardrails: a SKILL.md invariant doc, a regression test for escapeScript's </script>/<!-- neutralization, and a non-blocking PostToolUse hook that warns on hand-built inline <script>/raw-HTML sinks in Pro RSC source.

Overview

• The escapeScript regression test (injectRSCPayload.test.ts) is solid — it verifies the actual breakout string is absent, the neutralized closing-tag/comment-open forms are present, and the payload content is preserved (not just dropped). Good test.
• rsc-guardrails-check.sh is a substantial (271-line) regex/state-machine bash script for an advisory hook, but it's unusually well tested — rscGuardrailsHook.test.ts covers 28+ cases including multiline/prettier-split sinks, compound/logical assignment operators, type-only declarations, and collision/dedup cases. No shell-injection risk: the file path is always used quoted and never eval'd.
• The hook always exits 0 (non-blocking) and is wired correctly into .claude/settings.json under the existing Edit|Write PostToolUse matcher, after autofix-file.

Findings (left as inline comments, all low severity — none block merge)

1. The "MAINTENANCE CONTRACT" comment claims the hook's own test suite "will catch drift" if injectRSCPayload.ts is reformatted, but the test suite exercises hardcoded literal strings duplicated from the hook script, not the real file — so it can't actually catch that drift. Today the SAFE_*_LINE constants do exactly match the real file (verified), but the safety-net claim is inaccurate.
2. The hook's scan scope (packages/react-on-rails-pro/src/*.ts and *.tsx, plus one Ruby file) doesn't cover the base-package escaping helpers or the Node renderer that SKILL.md itself calls out as part of the same trust boundary — edits there get no warning.
3. Without jq installed, the hook silently no-ops on every real (stdin-JSON) invocation with no signal that the check isn't running.

None of these affect correctness of the shipped escapeScript fix/test itself, and the hook is advisory-only, so these are polish items rather than blockers.