fix: allow unsafe-eval in CSP for DynamicComponentRenderer#3107
fix: allow unsafe-eval in CSP for DynamicComponentRenderer#3107robert-inkeep merged 2 commits intomainfrom
Conversation
react-runner uses new Function() to compile and render dynamic component code at runtime. The CSP script-src directive excluded unsafe-eval in production, silently blocking execution — rendering an empty div. unsafe-eval is appropriate here because: - This is an authenticated admin dashboard, not a public-facing site - DynamicComponentRenderer is by design an eval engine for user-authored code - unsafe-inline is already allowed, and PostHog scripts load from external domains - The code being eval'd is authored by the same authenticated user
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: bce38a6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Clean, well-scoped fix. The CSP already permits 'unsafe-inline' and external script hosts, and this is an authenticated admin dashboard — adding 'unsafe-eval' doesn't meaningfully widen the attack surface. react-runner's useRunner relies on new Function() under the hood, so this is required for DynamicComponentRenderer to work in production.
Claude Opus ｜ 𝕏
|
TL;DR — Allows Key changes
Summary ｜ 2 files ｜ 2 commits ｜ base:
|
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Analysis
This PR adds 'unsafe-eval' to the production CSP to enable react-runner's new Function() calls for dynamic component rendering in the DynamicComponentRenderer.
Security context assessment:
| Factor | Assessment |
|---|---|
| Authentication boundary | ✅ Manage UI requires session authentication (non-public paths gated in proxy()) |
| Existing CSP posture | CSP already includes 'unsafe-inline' — incremental risk is limited |
| Feature intent | By design an eval engine for admin-authored component code |
| Code sources | AI-generated previews + admin-editable code in playground |
Why the change is acceptable:
- Scoped to authenticated admins — The manage UI middleware requires a valid session for non-public paths
- Existing CSP already permissive —
'unsafe-inline'is already present, so'unsafe-eval'is incremental - Functional necessity —
react-runnerusesnew Function()internally; there's no CSP-compliant alternative without architectural changes (e.g., iframe isolation) - Intentional design — The
DynamicComponentRendererexists specifically to evaluate user-authored component code at runtime
💭 Consider (1) 💭
💭 1) proxy.ts:61 Document the security tradeoff
Issue: The 'unsafe-eval' directive is a security control relaxation that future maintainers should understand.
Why: A brief comment would help future engineers understand why this is necessary and what the security implications are.
Fix: Consider adding a comment above line 61 (if the team decides documentation value outweighs the AGENTS.md "No Comments" guideline for security-critical configuration):
// SECURITY: unsafe-eval required for react-runner's new Function() calls
// in DynamicComponentRenderer. Scoped to authenticated admin users only.
"'unsafe-eval'",Note: This conflicts with the project's AGENTS.md "No Comments" convention — team discretion on whether security config warrants an exception.
Refs: MDN CSP script-src unsafe-eval
✅ APPROVE
Summary: The change is correct, minimal, and well-justified. The security risk is appropriately scoped to authenticated admin users, and the CSP already permits 'unsafe-inline'. The react-runner library requires 'unsafe-eval' for its new Function() internals — this is a functional necessity for the dynamic component preview feature. No blocking issues.
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-appsec |
1 | 0 | 1 | 0 | 0 | 0 | 0 |
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 1 | 0 | 1 | 0 | 0 | 0 | 0 |
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Delta Review
This is a re-review scoped to changes since the prior automated review.
Changes since last review:
- Added
.changeset/clever-magenta-bobolink.md— correctly formatted patch changeset for@inkeep/agents-manage-ui
The changeset is properly formatted, correctly identifies the package (@inkeep/agents-manage-ui), uses the appropriate bump level (patch), and has a clear description matching the PR's purpose.
The core CSP change ('unsafe-eval' addition in proxy.ts) was reviewed and approved in the prior review cycle. No additional findings.
✅ APPROVE
Summary: Clean delta — only a well-formed changeset was added since the last review. The core CSP change was already analyzed and approved. Ship it! 🚀
Reviewers (0)
No sub-reviewers dispatched — delta contained only a changeset file (no code changes to analyze).
Preview URLsUse these stable preview aliases for testing this PR:
These point to the same Vercel preview deployment as the bot comment, but they stay stable and easier to find. Raw Vercel deployment URLs
|
Ito Test Report ✅10 test cases ran. 1 additional finding, 9 passed. Overall, 9 of 10 tests passed, confirming expected security and routing behavior (CSP with script-src including unsafe-eval on public, protected, and redirect responses), stable deep-link and mobile render-panel behavior, correct unauthenticated redirect-to-login protection, and successful runtime component rendering in data/artifact editors and the agent playground chat path. The key issue found is one medium-severity bug where invalid JSX in the data component Render tab can fail before the in-component ErrorBoundary is mounted, leaving an effectively blank preview instead of a clear fallback error message. ✅ Passed (9)
ℹ️ Additional Findings (1)
🟠 Invalid render code surfaces fallback alert
Relevant code:
return js`
import { ErrorBoundary } from "react-error-boundary";
${code}
export default function App() {
const props = ${JSON.stringify(props)};
return (
<ErrorBoundary fallbackRender={fallbackRender}>
<${componentName} {...props} />
</ErrorBoundary>
);
}`;
const { element } = useRunner({
code: transformedCode,
scope,
});
return <div>{element}</div>;
<TabsContent value="render">
<div className="p-4">
<DynamicComponentRenderer code={renderCode} props={renderData} />
</div>
</TabsContent>Commit: Tell us how we did: Give Ito Feedback |
Summary
DynamicComponentRendererusesreact-runnerwhich callsnew Function()to compile and render user-authored component code at runtimescript-srcdirective excludedunsafe-evalin production, silently blocking execution — components rendered as empty<div></div>in prod while working locallyunsafe-evalsince this is an authenticated admin dashboard that already permitsunsafe-inlineand external scripts, and the component is by design an eval engine for user-authored codeTest plan
DynamicComponentRendererrenders in production (currently broken — empty div)unsafe-eval)