From c39ca87fe4c775c515e6d729379946af8744f3df Mon Sep 17 00:00:00 2001 From: Simple Agent Date: Wed, 30 Sep 2026 10:19:52 +0100 Subject: [PATCH] simple: fix #1670 --- CONTRIBUTING.md | 20 +++++- frontend/.eslintrc.js | 11 ++++ frontend/README.md | 7 +++ frontend/docs/styling.md | 131 +++++++++++++++++++++++++++++++++++++++ frontend/package.json | 3 +- 5 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 frontend/.eslintrc.js create mode 100644 frontend/README.md create mode 100644 frontend/docs/styling.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 79c56422..7698a0eb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -311,6 +311,24 @@ This is the **oldest** Rust toolchain version the c --- +## Code Style + +### Frontend Styling + +Frontend styling conventions are documented in +[`frontend/docs/styling.md`](frontend/docs/styling.md). Please read that +document before making UI changes. + +**No inline styles.** Do not use the `style` prop on JSX elements. All +styling must go through the project's approved mechanisms (e.g. Tailwind +utility classes, CSS Modules, or the shared design-system components). +Inline styles bypass theming, break static analysis, and make visual +regression testing unreliable. If you believe an inline style is +unavoidable, raise it in the PR description and get maintainer sign-off +before merging. + +--- + ## Security Please **do not** report security vulnerabilities through public GitHub issues, @@ -323,4 +341,4 @@ repository: This is the verified, monitored channel for security reports. See [`SECURITY.md`](SECURITY.md) for the full disclosure policy and response -timelines. +timelines. \ No newline at end of file diff --git a/frontend/.eslintrc.js b/frontend/.eslintrc.js new file mode 100644 index 00000000..499dc711 --- /dev/null +++ b/frontend/.eslintrc.js @@ -0,0 +1,11 @@ +module.exports = { + extends: ['next/core-web-vitals'], + rules: { + 'react/forbid-dom-props': [ + 'error', + { + forbid: ['style'], + }, + ], + }, +}; \ No newline at end of file diff --git a/frontend/README.md b/frontend/README.md new file mode 100644 index 00000000..4f6807e0 --- /dev/null +++ b/frontend/README.md @@ -0,0 +1,7 @@ +# Frontend + +This directory contains the frontend application. + +## Documentation + +- [Styling conventions](../docs/styling.md) — CSP-safe styling rules and conventions for frontend contributors. \ No newline at end of file diff --git a/frontend/docs/styling.md b/frontend/docs/styling.md new file mode 100644 index 00000000..52a5e675 --- /dev/null +++ b/frontend/docs/styling.md @@ -0,0 +1,131 @@ +# Styling Guide + +## CSP Constraint: Inline `style` Props Are Not Allowed + +This project runs under a strict **Content Security Policy (CSP)** in production. The policy does **not** include `'unsafe-inline'` for styles, which means: + +- Inline `style` attributes on DOM elements are **stripped or blocked** by the browser. +- React's `style={{ ... }}` prop compiles down to an inline `style` attribute, so it is affected by the same restriction. +- The failure is **silent**: no exception is thrown, no console error is guaranteed, and the element simply renders without the intended styles. This makes it easy to ship a broken UI without noticing during development (where CSP is often relaxed). + +Because of this, **do not use the `style` prop** on DOM elements or on components that forward it to the DOM. Use CSS classes or CSS modules instead. + +## Required Pattern: CSS Classes / CSS Modules + +The migration established the following pattern for all styling: + +1. **Static styles** live in a stylesheet (global CSS or a CSS module). +2. **Dynamic styles** are expressed by toggling **class names**, not by computing inline style objects. +3. Components receive class names via the `className` prop. + +### Incorrect + +```tsx +// ❌ Inline style prop — stripped/blocked by CSP in production. +function ProgressBar({ percent }: { percent: number }) { + return ( +
+ ); +} +``` + +```tsx +// ❌ Passing a style object through to a DOM element. +function Badge({ color, children }: { color: string; children: React.ReactNode }) { + return {children}; +} +``` + +### Correct + +Use a CSS module (or global class) and toggle classes: + +```css +/* ProgressBar.module.css */ +.progress { + height: 4px; + background: var(--color-accent); +} + +.width0 { width: 0%; } +.width25 { width: 25%; } +.width50 { width: 50%; } +.width75 { width: 75%; } +.width100 { width: 100%; } +``` + +```tsx +// ✅ Class-based styling — CSP-safe. +import styles from "./ProgressBar.module.css"; + +const WIDTH_CLASS: Record = { + 0: styles.width0, + 25: styles.width25, + 50: styles.width50, + 75: styles.width75, + 100: styles.width100, +}; + +function ProgressBar({ percent }: { percent: number }) { + const widthClass = WIDTH_CLASS[percent] ?? styles.width0; + return
; +} +``` + +```tsx +// ✅ Variant classes instead of inline color. +import styles from "./Badge.module.css"; + +function Badge({ variant, children }: { variant: "info" | "warn"; children: React.ReactNode }) { + return {children}; +} +``` + +For values that are genuinely dynamic and cannot be enumerated as classes, prefer: + +- A CSS custom property set via a class or a small set of classes, or +- A dedicated stylesheet rule keyed off a `data-*` attribute, or +- A CSS-in-JS solution that emits a real stylesheet (not inline attributes). + +If you believe you have a case that truly requires an inline style, raise it for review before adding one — it will not work in production. + +## Enforcement Gap + +There is currently **no lint rule** that catches inline `style` props. This is why the CSP violations were able to land in the first place. To prevent regressions, add the following ESLint rules: + +```jsonc +// .eslintrc (excerpt) +{ + "rules": { + "react/forbid-dom-props": [ + "error", + { "forbid": [{ "propName": "style", "message": "Inline style props are blocked by CSP. Use a CSS class or CSS module instead." }] } + ], + "react/forbid-component-props": [ + "error", + { "forbid": [{ "propName": "style", "message": "Inline style props are blocked by CSP. Use a CSS class or CSS module instead." }] } + ] + } +} +``` + +`react/forbid-dom-props` covers intrinsic elements (`
`), while `react/forbid-component-props` covers custom components that may forward `style` to the DOM. Both are needed to close the gap. + +## Historical Context + +This constraint was discovered the hard way. Two commits fixed the fallout from inline styles that were silently dropped under CSP: + +- **`5bd5e51`** — initial fix replacing inline `style` props with CSS classes. +- **`e80a15b`** — follow-up cleanup covering remaining components and edge cases. + +These commits are the reference for the pattern described above. When in doubt, look at how those changes were structured. + +## Summary + +- **Never** use the `style` prop on DOM elements or components that forward it to the DOM. +- Use **CSS classes / CSS modules** and toggle `className` for dynamic styling. +- The failure mode is **silent** — it will not show up in dev if CSP is relaxed. +- Add `react/forbid-dom-props` and `react/forbid-component-props` to enforce this automatically. \ No newline at end of file diff --git a/frontend/package.json b/frontend/package.json index af613531..616c5caf 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -59,6 +59,7 @@ "eslint": "^9.20.0", "eslint-config-next": "16.3.3", "eslint-config-prettier": "^10.1.8", + "eslint-plugin-react": "^7.37.4", "jest": "^30.4.2", "jest-axe": "^10.0.0", "jest-environment-jsdom": "^30.5.0", @@ -72,4 +73,4 @@ "node": ">=18.0.0", "npm": ">=9.0.0" } -} +} \ No newline at end of file