feat(css): add CSP nonce to hono/css related style and script tags

meck93 commented 1 week ago

closes #3694

This PR extends upon the work of https://github.com/honojs/hono/pull/2577 with the goal to bring the CSS nonce to the inline style and script tags created by the usage of hono/css. The goal is to be able to add <Style nonce={nonce} /> in order to comply with strict CSP rules.

The author should do the following, if applicable

[x] Add tests
[x] Run tests
[x] bun run format:fix && bun run lint:fix to format the code

To Do

[ ] There are still two test failures related to <script nonce="1234">document.querySelector('#hono-css').textContent+="..."</script> being added to the snapshot which should end up there. One should test if this is only due to the test setup and the <script> tag not being executed in the test environment.

@usualoma could you provide me with some pointers on where to start further debugging these test failures? I see that you've implemented most code around CSP and CSS.

 FAIL  src/helper/css/index.test.tsx > CSS Helper > render css > Booleans, Null, and Undefined Are Ignored > Should render CSS styles with CSP nonce
AssertionError: expected '<script nonce="1234">document.querySe…' to be '<style id="hono-css" nonce="1234">.cs…' // Object.is equality

Expected: "<style id="hono-css" nonce="1234">.css-2458908649{background-color:blue}</style><h1 class="css-2458908649">Hello!</h1>"
Received: "<script nonce="1234">document.querySelector('#hono-css').textContent+=".css-2458908649{background-color:blue}"</script><style id="hono-css" nonce="1234"></style><h1 class="css-2458908649">Hello!</h1>"

 ❯ src/helper/css/common.case.test.tsx:502:42
    500|           </>
    501|         )
    502|         expect(await toString(template)).toBe(
       |                                          ^
    503|           '<style id="hono-css" nonce="1234">.css-2458908649{background-color:blue}</style><h1 class="css-2458908649">Hello!</h1>'
    504|         )

 FAIL  src/helper/css/index.test.tsx > CSS Helper > with `html` tag function > Should render CSS styles with `html` tag function and CSP nonce
AssertionError: expected '<script nonce="1234">document.querySe…' to be '<style id="hono-css" nonce="1234">.cs…' // Object.is equality

- Expected
+ Received

- <style id="hono-css" nonce="1234">.css-2458908649{background-color:blue}</style>
+ <script nonce="1234">document.querySelector('#hono-css').textContent+=".css-2458908649{background-color:blue}"</script><style id="hono-css" nonce="1234"></style>
          <h1 class="css-2458908649">Hello!</h1>

 ❯ src/helper/css/index.test.tsx:68:40
     66|       const template = html`${Style({ nonce: '1234' })}
     67|         <h1 class="${headerClass}">Hello!</h1>`
     68|       expect(await toString(template)).toBe(
       |                                        ^
     69|         `<style id="hono-css" nonce="1234">.css-2458908649{background-color:blue}</style>
     70|         <h1 class="css-2458908649">Hello!</h1>`

[ ] Add TSDoc/JSDoc to document the code

usualoma commented 1 week ago

Hi @meck93, Thank you for making the pull request! I understand what you want to do and think your approach is very good. I haven't had time to answer it, but I will be able to answer the test in the next few days, so please wait a bit.

usualoma commented 6 days ago

Hi @meck93 , sorry to keep you waiting.

I've created a pull request, so please check it out. https://github.com/meck93/hono/pull/1

The test failure is fixed in the following commit. https://github.com/meck93/hono/pull/1/commits/2f06776d418a77c42b0d9e0956b9b0d97b35a7c4

Also, context is an arbitrary object, but the internal specification does not allow its updating, so I have changed the code to use it as a key in a WeakMap.

Thank you.

meck93 commented 6 days ago

Hi @meck93 , sorry to keep you waiting.

I've created a pull request, so please check it out. meck93#1

The test failure is fixed in the following commit. meck93@2f06776

Also, context is an arbitrary object, but the internal specification does not allow its updating, so I have changed the code to use it as a key in a WeakMap.

Thank you.

Thanks a lot for the feedback and the pull request. I like your approach using the nonceMap (really clean and easy to understand also if you're unfamiliar with the rest of the hono code). I've merged your PR ✅

usualoma commented 6 days ago

Hi @meck93, Thank you. LGTM!

codecov[bot] commented 5 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.70%. Comparing base (c8f6a86) to head (45ce947). Report is 7 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #3685 +/- ## ======================================= Coverage 91.70% 91.70% ======================================= Files 159 159 Lines 10145 10159 +14 Branches 2860 2871 +11 ======================================= + Hits 9303 9316 +13 - Misses 840 842 +2 + Partials 2 1 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

yusukebe commented 5 days ago

@meck93

Thank you for the PR. I like this feature. This is a feat commit, but the change is slight, so I'll include this in the next patch release.

And can you create a PR to add the description of this nonce attribute to our website, though it will be short?

meck93 commented 4 days ago

https://hono.dev/docs/helpers/css

@yusukebe Sure. Here you go: https://github.com/honojs/website/pull/536 Let me know if you want something changed. Thanks!

honojs / hono