diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 79c56422..9ea85b2e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,6 +10,7 @@ Thank you for your interest in contributing! This guide covers everything you ne - [Pull Request Process](#pull-request-process) - [Running Tests](#running-tests) - [Code Style](#code-style) +- [Frontend Styling (CSP-Safe)](#frontend-styling-csp-safe) - [Security](#security) --- @@ -307,20 +308,60 @@ that at minimum covers: ## Minimum Supported Rust Version (MSRV) The `services/api` crate declares a `rust-version` field in its `Cargo.toml`. -This is the **oldest** Rust toolchain version the c +This is the **oldest** Rust --- -## Security +## Frontend Styling (CSP-Safe) + +### The rule: no inline `style` props + +Do **not** use the `style={{ ... }}` prop on React/Next.js components for +anything that affects rendering. Use CSS classes (or CSS modules) instead. + +```tsx +// ❌ Don't — CSP strips this at runtime, so the style silently disappears +
…
+ +// ✅ Do — put the rules in a stylesheet and reference them by class +
…
+``` + +### Why (this is not a style preference) -Please **do not** report security vulnerabilities through public GitHub issues, -discussions, or pull requests. +The frontend is served under a strict **Content-Security-Policy** that does not +allow inline styles. When a component sets `style={{ ... }}`, the browser +**blocks or strips** the resulting inline style attribute — the element renders +with no styling at all. This is a real production bug that was fixed in +`5bd5e51` (AppShell chrome) and `e80a15b` (all remaining inline `style` props), +and it is easy to reintroduce by accident because the code still *looks* +correct in the editor and in tests that don't enforce CSP. -Instead, report them privately via **GitHub Security Advisories** for this -repository: +Because the failure is silent (no error, just missing styles), the convention +below is mandatory rather than optional. -- https://github.com/solutions-plug/predictIQ/security/advisories/new +### Required pattern + +- Define styles in CSS (global stylesheet or a CSS module) and apply them via + `className`. +- For dynamic values, prefer a class variant (e.g. `panel--error`) or a CSS + custom property set through a class, rather than a `style` prop. +- If a value genuinely cannot be expressed as a class, raise it in the PR + description so the CSP implications can be reviewed — do not add an inline + `style` prop silently. + +### Enforcement + +There is currently **no lint rule or CI check** that blocks inline `style` +props, so this convention relies on review. Adding an ESLint rule such as +[`react/forbid-dom-props`](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/forbid-dom-props.md) +configured to forbid `style` is recommended and tracked separately; until it +lands, reviewers should flag any new `style={{ ... }}` prop. + +--- + +## Security -This is the verified, monitored channel for security reports. See -[`SECURITY.md`](SECURITY.md) for the full disclosure policy and response -timelines. +If you discover a security vulnerability, please **do not** open a public issue. +Instead, report it privately to the maintainers so it can be triaged and fixed +before disclosure. diff --git a/frontend/README.md b/frontend/README.md new file mode 100644 index 00000000..ff2e553e --- /dev/null +++ b/frontend/README.md @@ -0,0 +1,52 @@ +# Frontend + +This package contains the Handsoff web frontend. + +## Styling conventions (CSP-safe, no inline styles) + +### Why: the CSP constraint + +Our production Content-Security-Policy does **not** allow inline styles. As a +result, any `style={{ ... }}` prop passed to a component is silently dropped by +the browser — the element renders with no styling at all, and the bug is easy to +miss in development where the policy may be relaxed. + +This was a real production incident: commits `5bd5e51` ("move AppShell chrome +off inline styles, CSP was dropping them") and `e80a15b` ("migrate every +remaining inline style prop to CSS classes") fixed it by removing inline styles +entirely. The constraint is documented here so the same bug is not reintroduced. + +### The rule + +**Do not use the `style` prop for anything that affects rendering.** + +```jsx +// ❌ Wrong — stripped by CSP, element renders unstyled +
…
+ +// ✅ Correct — use a CSS class +
…
+``` + +Use CSS classes (or CSS modules) for all styling. Dynamic values that would +normally be computed inline should be expressed as a class variant, a CSS custom +property set through a class, or a data attribute selected in CSS — never as an +inline `style` prop. + +### Enforcement + +There is currently **no lint rule or CI check** enforcing this, so it relies on +review. We should add one: an ESLint rule such as +[`react/forbid-dom-props`](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/forbid-dom-props.md) +configured to forbid the `style` prop would catch regressions at lint time: + +```json +{ + "rules": { + "react/forbid-dom-props": ["error", { "forbid": ["style"] }] + } +} +``` + +Until that rule (or an equivalent CI check) lands, treat any new `style` prop as +a review blocker. diff --git a/frontend/docs/styling.md b/frontend/docs/styling.md new file mode 100644 index 00000000..21f070ed --- /dev/null +++ b/frontend/docs/styling.md @@ -0,0 +1,60 @@ +# Frontend Styling Conventions + +## The rule: no inline `style` props + +Do **not** use the `style` prop on React components or DOM elements for anything +that affects rendering: + +```jsx +// ❌ Don't do this +
…
+``` + +Instead, express styling through CSS classes (or CSS modules) and apply them via +`className`: + +```jsx +// ✅ Do this +
…
+``` + +## Why: Content Security Policy strips inline styles + +Our production Content Security Policy does not allow inline styles. When the +browser enforces that policy, inline `style` attributes are **silently dropped** — +the element still renders, but without the styling. This is not a build error and +not a test failure; it only shows up in production, which is exactly why it went +unnoticed for so long. + +This was a real production bug. Two commits fixed it: + +- `5bd5e51` — moved AppShell chrome off inline styles (CSP was dropping them) +- `e80a15b` — migrated every remaining inline `style` prop to CSS classes + +Because the failure mode is silent and environment-specific, the constraint is +easy to reintroduce by accident. That is why it is documented here: so a future +PR does not ship the same bug again. + +## The required pattern + +- Put styling in CSS (global stylesheets or CSS modules) and reference it with + `className`. +- For dynamic values that genuinely must vary at runtime, prefer toggling a + class (e.g. `className={isActive ? 'is-active' : ''}`) or setting a CSS custom + property through a class rather than writing an inline `style` object. +- If you believe you have a case that truly requires an inline style, raise it in + the PR description first — do not add one silently. + +## Enforcement + +There is currently **no** lint rule or CI check that blocks inline `style` props, +so nothing mechanically prevents a regression. Adding one is recommended: + +- ESLint's [`react/forbid-dom-props`](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/forbid-dom-props.md) + can be configured with `forbid: ['style']` to flag inline `style` props on DOM + elements. +- For components that forward a `style` prop, `react/forbid-component-props` + covers the same case. + +Until such a rule is enabled, this convention is enforced by review. Please keep +it in mind when reviewing frontend changes.