fix(csp): restaurer le positionnement des toasts (feuille layered, sans épingler de hash) - #257
Merged
Merged
Conversation
…ions
sonner injects its stylesheet at module evaluation time, and does it in the
worst possible order: it appends an EMPTY <style> element, then fills it. That
is one element and two CSP evaluations, hence exactly the two `inline-style`
violations reported on every page — <Toaster /> is mounted unconditionally in
the locale layout. Both hashes were confirmed by re-hashing the literal
extracted from the installed package.
The console noise was the visible half. The real defect: with the stylesheet
blocked, `[data-sonner-toaster]{position:fixed}` never applied, so toasts were
unpositioned in production. Measured against the live site: 2 violations and
`position: static`. After this change, on a production build: 0 violations and
`position: fixed`.
sonner exposes no nonce API, so the fix has two distinct halves:
- `@import 'sonner/dist/styles.css' layer(sonner)` serves the CSS from a 'self'
stylesheet. This is what makes the UI correct, and keeps it correct if the
hash ever drifts.
- the two hashes in `style-src` silence the runtime injection.
The import is layered on purpose. sonner ships its stylesheet UNLAYERED, and
unlayered CSS beats layered CSS whatever the specificity, so a naive import
would have overridden the Tailwind utilities styling our toasts (bg-card,
border-border, border-danger) in both themes. Declaring the layer order before
the imports puts sonner between base and components, so our utilities keep the
last word while sonner keeps its positioning rules.
The hashes are emitted in every environment, not just production: CSP3 makes a
directive ignore 'unsafe-inline' as soon as a nonce is present, so the dev
relaxation is a no-op for style-src and the violation reproduces in dev too.
The constants live in a module with zero imports because proxy.ts cannot be
imported from Vitest — it pulls in env.ts, which throws at module load without
the env vars. The unit test proves the constant is RIGHT by re-extracting and
re-hashing sonner's literal on every run, and refuses to go green if that
extraction breaks. The e2e assertion proves the constant is WIRED into the
emitted directive, which a unit test alone cannot show.
No 'unsafe-hashes': these are <style> elements, not inline style attributes.
package.json is untouched — sonner was already a dependency.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @thierryvm, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
…ply it security-auditor put a NO-GO on the first version of this branch, and it was right: the two halves of the diff contradicted each other. Allow-listing a <style> hash does not merely tolerate the element, it APPLIES its CSS. sonner's injected copy is unlayered, and unlayered CSS outranks every cascade layer whatever the specificity — so the hashes cancelled exactly the protection the @layer in the same diff had just installed. sonner defaults to theme: 'light' and <Toaster /> passes no theme, so its --normal-bg: #fff would have beaten bg-card and painted a white toast in dark mode, losing the severity colour coding with it. Two cosmetic console warnings are the cheaper defect. Keeping the hashes out also means a future compromised sonner release stays blocked by the CSP: only the vendored, layered copy — visible in a lockfile diff — can apply. What remains is the half that actually fixes the user-visible bug: the layered 'self' import, which restores `[data-sonner-toaster]{position:fixed}`. Measured on a production build, both themes: before (live site) 2 violations, position: static, dark bg rgb(17,26,46) after 2 violations, position: fixed, dark bg rgb(17,26,46) Positioning fixed, our theme tokens preserved in both modes — which the hashed version would not have achieved. The decision is documented where it will be re-read (globals.css) and locked by an e2e assertion that fails if any hash source reappears in style-src. The drift guard went away with the hashes it protected; the auditor also noted its failure message invited re-hashing a potentially hostile stylesheet.
thierryvm
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptôme
Deux violations CSP
inline-stylepar page en production, relevées par @Thierry.Diagnostic — deux hypothèses à moi, deux réfutations par les agents
1.
plan-reviewera réfuté ma piste Radix /get-nonce: ce singleton remplit son<style>avant de l'insérer, il ne peut donc pas produire un hash de chaîne vide. Il a désignésonner. Vérifié par re-calcul depuis le paquet installé — match exact sur les deux :sha256-47DEQpj8…<style>inséré vide par sonnersha256-CIxDM5jn…sonner/dist/index.mjs(14 859 car.)sonnerinsère un<style>vide puis le remplit : un élément, deux évaluations CSP.<Toaster />est monté sans condition dans le layout → violations sur chaque page.Le bug utilisateur que ça cachait : le CSS bloqué,
[data-sonner-toaster]{position:fixed}ne s'appliquait pas — les toasts sont mal positionnés en production.2.
security-auditora posé un NO-GO sur ma première version, qui épinglait les deux hashes pour faire taire la console. Motif décisif : autoriser un<style>, ce n'est pas le tolérer, c'est l'appliquer. La copie injectée est non-layered, donc elle l'emporte sur n'importe quelle couche quelle que soit la spécificité — les hashes annulaient exactement la protection@layerque le même diff installait. Et sonner utilisanttheme: 'light'par défaut, son--normal-bg: #fffaurait gagné surbg-card→ toast fond blanc en thème sombre, code couleur de sévérité perdu.Ce qui est livré
Un seul changement fonctionnel :
@import 'sonner/dist/styles.css' layer(sonner);avec ordre de couches déclaré avant les imports. Le CSS arrive par une source'self', les toasts retrouvent leur positionnement, et nos utilitaires Tailwind gardent le dernier mot.Pas de hash épinglé. Arbitrage assumé : deux avertissements console cosmétiques valent mieux qu'une régression visuelle en thème sombre. Bénéfice secondaire relevé par l'audit — une future version compromise de sonner reste bloquée par la CSP ; seule la copie vendored, layered et visible dans un diff de lockfile, s'applique.
Preuve mesurée (build de production, les deux thèmes)
style-srcstatic✗rgb(17, 26, 46)fixed✓rgb(17, 26, 46)✓Verrou
e2e/security-headers.spec.tséchoue si un hash ('sha256-…'/'sha384-…'/'sha512-…') ou'unsafe-hashes'réapparaît dansstyle-src. La décision est documentée danssrc/app/globals.css, là où elle sera relue.package.jsoninchangé —sonnerétait déjà une dépendance.Definition of DONE
Rapport complet :
docs/prs/PR-csp-sonner-report.mdnpm run lint0 erreur ·npm run typecheck0 erreur ·npm run test1645/1645plan-reviewer: 🔴 puis 🟡 APPROVED WITH CHANGES — 7 corrections appliquéessecurity-auditor: 🔴 NO-GO levé par l'option (a) qu'il recommandait